[llvm] [RISCV] Support FrameIndex operands in getMemOperandsWithOffsetWidth / getMemOperandWithOffsetWidth (PR #73802)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 29 07:08:11 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-risc-v
Author: Alex Bradbury (asb)
<details>
<summary>Changes</summary>
I noted AArch64 happily accepts a FrameIndex operand as well as a register. This doesn't cause any changes outside of my C++ unit test for the current state of in-tree, but this will cause additional test changes if #<!-- -->73789 is rebased on top of it.
---
I do have some caution here that the returned Offset doesn't seem at all as meaningful if you have a FrameIndex base. Though as I say, AArch64 has been doing this for some time - see https://reviews.llvm.org/D54847. I believe this change won't harm the approach taken in shouldClusterMemOps because memOpsHaveSameBasePtr will only return true if the FrameIndex operand is the same for both operations. (Though reviewers, please double-check you agree). This patch is directly applyable, so I've opted not to base it on top of #<!-- -->73789.
---
Full diff: https://github.com/llvm/llvm-project/pull/73802.diff
2 Files Affected:
- (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+1-1)
- (modified) llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp (+7-2)
``````````diff
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 2918e5654db4f9f..900eede0c4ef24c 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2304,7 +2304,7 @@ bool RISCVInstrInfo::getMemOperandWithOffsetWidth(
// load/store instructions.
if (LdSt.getNumExplicitOperands() != 3)
return false;
- if (!LdSt.getOperand(1).isReg() || !LdSt.getOperand(2).isImm())
+ if ((!LdSt.getOperand(1).isReg() && !LdSt.getOperand(1).isFI()) || !LdSt.getOperand(2).isImm())
return false;
if (!LdSt.hasOneMemOperand())
diff --git a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
index 135d7dbb426e3c2..b4c96a9c2a62ce7 100644
--- a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
+++ b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
@@ -154,7 +154,6 @@ TEST_P(RISCVInstrInfoTest, GetMemOperandsWithOffsetWidth) {
Res = TII->getMemOperandsWithOffsetWidth(*MI, BaseOps, Offset,
OffsetIsScalable, Width, TRI);
- // TODO: AArch64 can handle this case, and we probably should too.
BaseOps.clear();
MMO = MF->getMachineMemOperand(MachinePointerInfo(),
MachineMemOperand::MOStore, 4, Align(4));
@@ -165,7 +164,13 @@ TEST_P(RISCVInstrInfoTest, GetMemOperandsWithOffsetWidth) {
.addMemOperand(MMO);
Res = TII->getMemOperandsWithOffsetWidth(*MI, BaseOps, Offset,
OffsetIsScalable, Width, TRI);
- EXPECT_FALSE(Res);
+ ASSERT_TRUE(Res);
+ ASSERT_EQ(BaseOps.size(), 1u);
+ ASSERT_TRUE(BaseOps.front()->isFI());
+ EXPECT_EQ(BaseOps.front()->getIndex(), 2);
+ EXPECT_EQ(Offset, 4);
+ EXPECT_FALSE(OffsetIsScalable);
+ EXPECT_EQ(Width, 4u);
}
} // namespace
``````````
</details>
https://github.com/llvm/llvm-project/pull/73802
More information about the llvm-commits
mailing list