-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[RISCV] Remove stale comment from test. NFC #81098
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
Conversation
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesThe bug mentioned in the comment has been commited and did change the cfi_offset. Full diff: https://github.com/llvm/llvm-project/pull/81098.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 27037f4d5c5c85..405d99f14d8fd9 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -15812,7 +15812,8 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
MGN->getMemOperand(), IndexType, MGN->getExtensionType());
if (Index.getOpcode() == ISD::BUILD_VECTOR &&
- MGN->getExtensionType() == ISD::NON_EXTLOAD) {
+ MGN->getExtensionType() == ISD::NON_EXTLOAD &&
+ isTypeLegal(VT)) {
if (std::optional<VIDSequence> SimpleVID = isSimpleVIDSequence(Index);
SimpleVID && SimpleVID->StepDenominator == 1) {
const int64_t StepNumerator = SimpleVID->StepNumerator;
diff --git a/llvm/test/CodeGen/RISCV/zcmp-with-float.ll b/llvm/test/CodeGen/RISCV/zcmp-with-float.ll
index 93f95e9709b6a4..3eb91eb9a09c47 100644
--- a/llvm/test/CodeGen/RISCV/zcmp-with-float.ll
+++ b/llvm/test/CodeGen/RISCV/zcmp-with-float.ll
@@ -5,7 +5,6 @@
declare void @callee()
; Test the file could be compiled successfully.
-; .cfi_offset of fs0 is wrong here. It should be fixed by #66613.
define float @foo(float %arg) {
; RV32-LABEL: foo:
; RV32: # %bb.0: # %entry
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
MGN->getExtensionType() == ISD::NON_EXTLOAD && | ||
isTypeLegal(VT)) { |
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 think this is accidentally from #81088 (comment)
Otherwise we will crash since target intrinsics don't have their types legalized. Let the mgather get legalized first, then do the combine on the legal type. Fixes llvm#81098 Co-authored-by: <[email protected]>
7928c31
to
a0868bf
Compare
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
The bug mentioned in the comment has been commited and did change the cfi_offset.