[RISCV] add computeKnownBitsForTargetNode for RISCVISD::SRLW #155995
Merged
+144
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-backend-risc-v Author: Shreeyash Pandey (bojle) ChangesI've added support for computeKnownBitsForTargetNode for the SRLW instruction. A test has been included which uses the snippet of IR as suggested by @topperc. @RKSimon Full diff: https://github.com/llvm/llvm-project/pull/155995.diff 3 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 4a1db80076530..700a288e8f5c3 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -21276,6 +21276,7 @@ void RISCVTargetLowering::computeKnownBitsForTargetNode(const SDValue Op,
const APInt &DemandedElts,
const SelectionDAG &DAG,
unsigned Depth) const {
+ errs() << "computeKnownBitsForTargetNode\n";
unsigned BitWidth = Known.getBitWidth();
unsigned Opc = Op.getOpcode();
assert((Opc >= ISD::BUILTIN_OP_END ||
@@ -21332,6 +21333,7 @@ void RISCVTargetLowering::computeKnownBitsForTargetNode(const SDValue Op,
break;
}
case RISCVISD::SLLW: {
+ errs() << "computeKnownBitsForTargetNode SLLW\n";
KnownBits Known2;
Known = DAG.computeKnownBits(Op.getOperand(0), DemandedElts, Depth + 1);
Known2 = DAG.computeKnownBits(Op.getOperand(1), DemandedElts, Depth + 1);
@@ -21340,6 +21342,35 @@ void RISCVTargetLowering::computeKnownBitsForTargetNode(const SDValue Op,
Known = Known.sext(BitWidth);
break;
}
+ case RISCVISD::SRLW: {
+ errs() << "computeKnownBitsForTargetNode SRLW\n";
+ KnownBits Known2;
+ Known = DAG.computeKnownBits(Op.getOperand(0), DemandedElts, Depth + 1);
+ Known2 = DAG.computeKnownBits(Op.getOperand(1), DemandedElts, Depth + 1);
+ Known = KnownBits::lshr(Known.trunc(32), Known2.trunc(5).zext(32));
+ // Restore the original width by sign extending.
+ Known = Known.sext(BitWidth);
+ errs() << "Known bits (SRLW) " << Known << '\n';
+ unsigned BitWidth = Known.getBitWidth();
+ unsigned zeros = 0;
+ unsigned ones = 0;
+ unsigned unknown = 0;
+ for (unsigned I = 0; I < BitWidth; ++I) {
+ unsigned N = BitWidth - I - 1;
+ if (Known.Zero[N]) {
+ zeros++;
+ errs() << "0";
+ } else if (Known.One[N]) {
+ ones++;
+ errs() << "1";
+ } else {
+ unknown++;
+ errs() << "?";
+ }
+ }
+ errs() << "Zeros " << zeros << "Ones " << ones << "Unknown " << unknown << '\n';
+ break;
+ }
case RISCVISD::CTZW: {
KnownBits Known2 = DAG.computeKnownBits(Op.getOperand(0), Depth + 1);
unsigned PossibleTZ = Known2.trunc(32).countMaxTrailingZeros();
diff --git a/llvm/unittests/Target/RISCV/CMakeLists.txt b/llvm/unittests/Target/RISCV/CMakeLists.txt
index 8da8c3896faf1..701bbee55da71 100644
--- a/llvm/unittests/Target/RISCV/CMakeLists.txt
+++ b/llvm/unittests/Target/RISCV/CMakeLists.txt
@@ -19,4 +19,5 @@ set(LLVM_LINK_COMPONENTS
add_llvm_target_unittest(RISCVTests
MCInstrAnalysisTest.cpp
RISCVInstrInfoTest.cpp
+ RISCVSelectionDAGTest.cpp
)
diff --git a/llvm/unittests/Target/RISCV/RISCVSelectionDAGTest.cpp b/llvm/unittests/Target/RISCV/RISCVSelectionDAGTest.cpp
new file mode 100644
index 0000000000000..a13f88484c00c
--- /dev/null
+++ b/llvm/unittests/Target/RISCV/RISCVSelectionDAGTest.cpp
@@ -0,0 +1,109 @@
+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RISCVISelLowering.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/SelectionDAG.h"
+#include "llvm/CodeGen/TargetLowering.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Module.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/KnownBits.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
+#include "gtest/gtest.h"
+
+namespace llvm {
+
+class RISCVSelectionDAGTest : public testing::Test {
+
+protected:
+ static void SetUpTestCase() {
+ LLVMInitializeRISCVTargetInfo();
+ LLVMInitializeRISCVTarget();
+ LLVMInitializeRISCVTargetMC();
+ }
+
+ void SetUp() override {
+ StringRef Assembly = "define void @f() { ret void }";
+
+ Triple TargetTriple("riscv64", "unknown", "linux");
+
+ std::string Error;
+ const Target *T = TargetRegistry::lookupTarget("", TargetTriple, Error);
+
+ TargetOptions Options;
+ TM = std::unique_ptr<TargetMachine>(T->createTargetMachine(
+ TargetTriple, "generic", "", Options, std::nullopt, std::nullopt,
+ CodeGenOptLevel::Default));
+
+ SMDiagnostic SMError;
+ M = parseAssemblyString(Assembly, SMError, Context);
+ if (!M)
+ report_fatal_error(SMError.getMessage());
+ M->setDataLayout(TM->createDataLayout());
+
+ F = M->getFunction("f");
+ if (!F)
+ report_fatal_error("Function 'f' not found");
+
+ MachineModuleInfo MMI(TM.get());
+
+ MF = std::make_unique<MachineFunction>(*F, *TM, *TM->getSubtargetImpl(*F),
+ MMI.getContext(), /*FunctionNum*/ 0);
+
+ DAG = std::make_unique<SelectionDAG>(*TM, CodeGenOptLevel::None);
+ if (!DAG)
+ report_fatal_error("SelectionDAG allocation failed");
+
+ OptimizationRemarkEmitter ORE(F);
+ DAG->init(*MF, ORE, /*LibInfo*/ nullptr, /*AA*/ nullptr,
+ /*AC*/ nullptr, /*MDT*/ nullptr, /*MSDT*/ nullptr, MMI, nullptr);
+ }
+
+ LLVMContext Context;
+ std::unique_ptr<TargetMachine> TM;
+ std::unique_ptr<Module> M;
+ Function *F = nullptr;
+ std::unique_ptr<MachineFunction> MF;
+ std::unique_ptr<SelectionDAG> DAG;
+};
+
+/// SRLW: Logical Shift Right
+TEST_F(RISCVSelectionDAGTest, computeKnownBits_SRLW) {
+ // Following DAG is created from this IR snippet:
+ //
+ // define i64 @f(i32 %x, i32 %y) {
+ // %a = and i32 %x, 2147483647 ; zeros the MSB for %x
+ // %b = lshr i32 %a, %y
+ // %c = zext i32 %b to i64 ; makes the most significant 32 bits 0
+ // ret i64 %c
+ // }
+ SDLoc Loc;
+ auto IntVT = EVT::getIntegerVT(Context, 32);
+ auto Int64VT = EVT::getIntegerVT(Context, 64);
+ auto Px = DAG->getRegister(0, IntVT);
+ auto Py = DAG->getConstant(2147483647, Loc, IntVT);
+ auto N1 = DAG->getNode(ISD::AND, Loc, IntVT, Px, Py);
+ auto Qx = DAG->getRegister(0, IntVT);
+ auto N2 = DAG->getNode(ISD::SRL, Loc, IntVT, N1, Qx);
+ auto N3 = DAG->getNode(ISD::ZERO_EXTEND, Loc, Int64VT, N2);
+ // N1 = 0???????????????????????????????
+ // N2 = 0???????????????????????????????
+ // N3 = 000000000000000000000000000000000???????????????????????????????
+ // After zero extend, we expect 33 most significant zeros to be known:
+ // 32 from sign extension and 1 from AND operation
+ KnownBits Known = DAG->computeKnownBits(N3);
+ EXPECT_EQ(Known.Zero, APInt(64, -2147483648));
+ EXPECT_EQ(Known.One, APInt(64, 0));
+}
+
+} // end namespace llvm
|
RKSimon
reviewed
Aug 29, 2025
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
topperc
reviewed
Aug 29, 2025
mshockwave
reviewed
Aug 29, 2025
| TEST_F(RISCVSelectionDAGTest, computeKnownBits_SRLW) { | ||
| // Following DAG is created from this IR snippet: | ||
| // | ||
| // define i64 @f(i32 %x, i32 %y) { |
There was a problem hiding this comment.
you could also add a LIT codegen test instead of unittest using this LLVM IR as input. Personally I thought that would be more succinct
There was a problem hiding this comment.
I was trying to write a LIT test, but couldn't get my head around it. My rationale to write a unittest was that I am adding a small portion of code for Known Bits Analysis of the SRLW, so I have to check whether computeKnownBits for SRLW returns appropriately. Is there a way to test for the values of known bits through the llvm IR, so a lit test can be written? As I understand, a lit test would be able to check if the SRLW instruction has been generated or not (and in the expected format). This, I believe already happens in the RISCV codegen. Can you show me an example to understand this clearly?
There was a problem hiding this comment.
You can't test the KnownBits directly, but you can test that it enables optimizations. Here's the recent PR I wrote for sign bits of SRAW. #155564
There was a problem hiding this comment.
So, would you recommend I work on a lit test too? What about the current unittest (drop it/keep it)?
|
Please avoid force push unless necessary: https://llvm.org/docs/GitHub.html#rebasing-pull-requests-and-force-pushes |
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
topperc
reviewed
Aug 30, 2025
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
topperc
reviewed
Aug 31, 2025
topperc
reviewed
Aug 31, 2025
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
|
I've re-written this based on the output of 'llc'. The last part: is still redundant, but i've kept it for sake of imitating the generated DAG. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
RKSimon
requested changes
Sep 3, 2025
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
RKSimon
approved these changes
Sep 3, 2025
|
@bojle Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/139/builds/19390 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/12623 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/9537 Here is the relevant piece of the build log for the reference |
|
@bojle From the buildbot failures it looks like there's a missing lib dependency |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've added support for computeKnownBitsForTargetNode for the SRLW instruction. A test has been included which uses the snippet of IR as suggested by topperc.
Fixed #154913
@RKSimon