-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[RISCV] add computeKnownBitsForTargetNode for RISCVISD::SRLW #155995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
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
|
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
Signed-off-by: Shreeyash Pandey <shreeyash335@gmail.com>
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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