[llvm] [BOLT] Do not return Def-ed registers from MCPlusBuilder::getUsedRegs (PR #129890)

via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 5 07:34:35 PST 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Anatoly Trosinenko (atrosinenko)

<details>
<summary>Changes</summary>

Update the implementation of `MCPlusBuilder::getUsedRegs` to match its description in the header file, add unit tests.

---
Full diff: https://github.com/llvm/llvm-project/pull/129890.diff


2 Files Affected:

- (modified) bolt/lib/Core/MCPlusBuilder.cpp (+3-3) 
- (modified) bolt/unittests/Core/MCPlusBuilder.cpp (+122-18) 


``````````diff
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 7ff7a2288451c..20e786afc6ccf 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -442,10 +442,10 @@ void MCPlusBuilder::getUsedRegs(const MCInst &Inst, BitVector &Regs) const {
   for (MCPhysReg ImplicitUse : InstInfo.implicit_uses())
     Regs |= getAliases(ImplicitUse, /*OnlySmaller=*/true);
 
-  for (unsigned I = 0, E = Inst.getNumOperands(); I != E; ++I) {
-    if (!Inst.getOperand(I).isReg())
+  for (const MCOperand &Operand : useOperands(Inst)) {
+    if (!Operand.isReg())
       continue;
-    Regs |= getAliases(Inst.getOperand(I).getReg(), /*OnlySmaller=*/true);
+    Regs |= getAliases(Operand.getReg(), /*OnlySmaller=*/true);
   }
 }
 
diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp
index d367eb07f7767..df5bb0a3dfbd6 100644
--- a/bolt/unittests/Core/MCPlusBuilder.cpp
+++ b/bolt/unittests/Core/MCPlusBuilder.cpp
@@ -8,6 +8,7 @@
 
 #ifdef AARCH64_AVAILABLE
 #include "AArch64Subtarget.h"
+#include "MCTargetDesc/AArch64MCTargetDesc.h"
 #endif // AARCH64_AVAILABLE
 
 #ifdef X86_AVAILABLE
@@ -19,6 +20,7 @@
 #include "bolt/Rewrite/RewriteInstance.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/MC/MCInstBuilder.h"
 #include "llvm/Support/TargetSelect.h"
 #include "gtest/gtest.h"
 
@@ -70,16 +72,28 @@ struct MCPlusBuilderTester : public testing::TestWithParam<Triple::ArchType> {
                             BC->MRI.get(), BC->STI.get())));
   }
 
+  void assertRegMask(const BitVector &RegMask,
+                     std::initializer_list<MCPhysReg> ExpectedRegs) {
+    ASSERT_EQ(RegMask.count(), ExpectedRegs.size());
+    for (MCPhysReg Reg : ExpectedRegs)
+      ASSERT_TRUE(RegMask[Reg]) << "Expected " << BC->MRI->getName(Reg) << ".";
+  }
+
+  void assertRegMask(std::function<void(BitVector &)> FillRegMask,
+                     std::initializer_list<MCPhysReg> ExpectedRegs) {
+    BitVector RegMask(BC->MRI->getNumRegs());
+    FillRegMask(RegMask);
+    assertRegMask(RegMask, ExpectedRegs);
+  }
+
   void testRegAliases(Triple::ArchType Arch, uint64_t Register,
-                      uint64_t *Aliases, size_t Count,
+                      std::initializer_list<MCPhysReg> ExpectedAliases,
                       bool OnlySmaller = false) {
     if (GetParam() != Arch)
       GTEST_SKIP();
 
     const BitVector &BV = BC->MIB->getAliases(Register, OnlySmaller);
-    ASSERT_EQ(BV.count(), Count);
-    for (size_t I = 0; I < Count; ++I)
-      ASSERT_TRUE(BV[Aliases[I]]);
+    assertRegMask(BV, ExpectedAliases);
   }
 
   char ElfBuf[sizeof(typename ELF64LE::Ehdr)] = {};
@@ -94,17 +108,109 @@ INSTANTIATE_TEST_SUITE_P(AArch64, MCPlusBuilderTester,
                          ::testing::Values(Triple::aarch64));
 
 TEST_P(MCPlusBuilderTester, AliasX0) {
-  uint64_t AliasesX0[] = {AArch64::W0,    AArch64::W0_HI,
-                          AArch64::X0,    AArch64::W0_W1,
-                          AArch64::X0_X1, AArch64::X0_X1_X2_X3_X4_X5_X6_X7};
-  size_t AliasesX0Count = sizeof(AliasesX0) / sizeof(*AliasesX0);
-  testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count);
+  testRegAliases(Triple::aarch64, AArch64::X0,
+                 {AArch64::W0, AArch64::W0_HI, AArch64::X0, AArch64::W0_W1,
+                  AArch64::X0_X1, AArch64::X0_X1_X2_X3_X4_X5_X6_X7});
 }
 
 TEST_P(MCPlusBuilderTester, AliasSmallerX0) {
-  uint64_t AliasesX0[] = {AArch64::W0, AArch64::W0_HI, AArch64::X0};
-  size_t AliasesX0Count = sizeof(AliasesX0) / sizeof(*AliasesX0);
-  testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count, true);
+  testRegAliases(Triple::aarch64, AArch64::X0,
+                 {AArch64::W0, AArch64::W0_HI, AArch64::X0},
+                 /*OnlySmaller=*/true);
+}
+
+TEST_P(MCPlusBuilderTester, testAccessedRegsImplicitDef) {
+  if (GetParam() != Triple::aarch64)
+    GTEST_SKIP();
+
+  // adds x0, x5, #42
+  MCInst Inst = MCInstBuilder(AArch64::ADDSXri)
+                    .addReg(AArch64::X0)
+                    .addReg(AArch64::X5)
+                    .addImm(42)
+                    .addImm(0);
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getClobberedRegs(Inst, BV); },
+                {AArch64::NZCV, AArch64::W0, AArch64::X0, AArch64::W0_HI,
+                 AArch64::X0_X1_X2_X3_X4_X5_X6_X7, AArch64::W0_W1,
+                 AArch64::X0_X1});
+
+  assertRegMask(
+      [&](BitVector &BV) { BC->MIB->getTouchedRegs(Inst, BV); },
+      {AArch64::NZCV, AArch64::W0, AArch64::W5, AArch64::X0, AArch64::X5,
+       AArch64::W0_HI, AArch64::W5_HI, AArch64::X0_X1_X2_X3_X4_X5_X6_X7,
+       AArch64::X2_X3_X4_X5_X6_X7_X8_X9, AArch64::X4_X5_X6_X7_X8_X9_X10_X11,
+       AArch64::W0_W1, AArch64::W4_W5, AArch64::X0_X1, AArch64::X4_X5});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getWrittenRegs(Inst, BV); },
+                {AArch64::NZCV, AArch64::W0, AArch64::X0, AArch64::W0_HI});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getUsedRegs(Inst, BV); },
+                {AArch64::W5, AArch64::X5, AArch64::W5_HI});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getSrcRegs(Inst, BV); },
+                {AArch64::W5, AArch64::X5, AArch64::W5_HI});
+}
+
+TEST_P(MCPlusBuilderTester, testAccessedRegsImplicitUse) {
+  if (GetParam() != Triple::aarch64)
+    GTEST_SKIP();
+
+  // b.eq <label>
+  MCInst Inst =
+      MCInstBuilder(AArch64::Bcc)
+          .addImm(AArch64CC::EQ)
+          .addImm(0); // <label> - should be Expr, but immediate 0 works too.
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getClobberedRegs(Inst, BV); },
+                {});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getTouchedRegs(Inst, BV); },
+                {AArch64::NZCV});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getWrittenRegs(Inst, BV); }, {});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getUsedRegs(Inst, BV); },
+                {AArch64::NZCV});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getSrcRegs(Inst, BV); },
+                {AArch64::NZCV});
+}
+
+TEST_P(MCPlusBuilderTester, testAccessedRegsMultipleDefs) {
+  if (GetParam() != Triple::aarch64)
+    GTEST_SKIP();
+
+  // ldr x0, [x5], #16
+  MCInst Inst = MCInstBuilder(AArch64::LDRXpost)
+                    .addReg(AArch64::X5)
+                    .addReg(AArch64::X0)
+                    .addReg(AArch64::X5)
+                    .addImm(16);
+
+  assertRegMask(
+      [&](BitVector &BV) { BC->MIB->getClobberedRegs(Inst, BV); },
+      {AArch64::W0, AArch64::W5, AArch64::X0, AArch64::X5, AArch64::W0_HI,
+       AArch64::W5_HI, AArch64::X0_X1_X2_X3_X4_X5_X6_X7,
+       AArch64::X2_X3_X4_X5_X6_X7_X8_X9, AArch64::X4_X5_X6_X7_X8_X9_X10_X11,
+       AArch64::W0_W1, AArch64::W4_W5, AArch64::X0_X1, AArch64::X4_X5});
+
+  assertRegMask(
+      [&](BitVector &BV) { BC->MIB->getTouchedRegs(Inst, BV); },
+      {AArch64::W0, AArch64::W5, AArch64::X0, AArch64::X5, AArch64::W0_HI,
+       AArch64::W5_HI, AArch64::X0_X1_X2_X3_X4_X5_X6_X7,
+       AArch64::X2_X3_X4_X5_X6_X7_X8_X9, AArch64::X4_X5_X6_X7_X8_X9_X10_X11,
+       AArch64::W0_W1, AArch64::W4_W5, AArch64::X0_X1, AArch64::X4_X5});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getWrittenRegs(Inst, BV); },
+                {AArch64::W0, AArch64::X0, AArch64::W0_HI, AArch64::W5,
+                 AArch64::X5, AArch64::W5_HI});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getUsedRegs(Inst, BV); },
+                {AArch64::W5, AArch64::X5, AArch64::W5_HI});
+
+  assertRegMask([&](BitVector &BV) { BC->MIB->getSrcRegs(Inst, BV); },
+                {AArch64::W5, AArch64::X5, AArch64::W5_HI});
 }
 
 #endif // AARCH64_AVAILABLE
@@ -115,15 +221,13 @@ INSTANTIATE_TEST_SUITE_P(X86, MCPlusBuilderTester,
                          ::testing::Values(Triple::x86_64));
 
 TEST_P(MCPlusBuilderTester, AliasAX) {
-  uint64_t AliasesAX[] = {X86::RAX, X86::EAX, X86::AX, X86::AL, X86::AH};
-  size_t AliasesAXCount = sizeof(AliasesAX) / sizeof(*AliasesAX);
-  testRegAliases(Triple::x86_64, X86::AX, AliasesAX, AliasesAXCount);
+  testRegAliases(Triple::x86_64, X86::AX,
+                 {X86::RAX, X86::EAX, X86::AX, X86::AL, X86::AH});
 }
 
 TEST_P(MCPlusBuilderTester, AliasSmallerAX) {
-  uint64_t AliasesAX[] = {X86::AX, X86::AL, X86::AH};
-  size_t AliasesAXCount = sizeof(AliasesAX) / sizeof(*AliasesAX);
-  testRegAliases(Triple::x86_64, X86::AX, AliasesAX, AliasesAXCount, true);
+  testRegAliases(Triple::x86_64, X86::AX, {X86::AX, X86::AL, X86::AH},
+                 /*OnlySmaller=*/true);
 }
 
 TEST_P(MCPlusBuilderTester, ReplaceRegWithImm) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/129890


More information about the llvm-commits mailing list