[llvm] [BOLT] Do not return Def-ed registers from MCPlusBuilder::getUsedRegs (PR #129890)
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 5 07:24:54 PST 2025
https://github.com/atrosinenko created https://github.com/llvm/llvm-project/pull/129890
Update the implementation of `MCPlusBuilder::getUsedRegs` to match its description in the header file, add unit tests.
>From 1692dd6946c05b26ff23268e2577614bc1648c8a Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Wed, 5 Mar 2025 16:45:33 +0300
Subject: [PATCH 1/2] [BOLT] Do not return Def-ed registers from
MCPlusBuilder::getUsedRegs
Update the implementation of MCPlusBuilder::getUsedRegs to match its
description in the header file, add unit tests.
---
bolt/lib/Core/MCPlusBuilder.cpp | 6 +-
bolt/unittests/Core/MCPlusBuilder.cpp | 110 ++++++++++++++++++++++++++
2 files changed, 113 insertions(+), 3 deletions(-)
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..e0e8a1a4eb915 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,6 +72,20 @@ 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,
bool OnlySmaller = false) {
@@ -107,6 +123,100 @@ TEST_P(MCPlusBuilderTester, AliasSmallerX0) {
testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count, 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
#ifdef X86_AVAILABLE
>From 83334f0a5160c78a30c3b45631b8ca54b5c06183 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Wed, 5 Mar 2025 17:12:38 +0300
Subject: [PATCH 2/2] Update bolt/unittests/Core/MCPlusBuilder.cpp
---
bolt/unittests/Core/MCPlusBuilder.cpp | 30 +++++++++++----------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp
index e0e8a1a4eb915..df5bb0a3dfbd6 100644
--- a/bolt/unittests/Core/MCPlusBuilder.cpp
+++ b/bolt/unittests/Core/MCPlusBuilder.cpp
@@ -87,15 +87,13 @@ struct MCPlusBuilderTester : public testing::TestWithParam<Triple::ArchType> {
}
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)] = {};
@@ -110,17 +108,15 @@ 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) {
@@ -225,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) {
More information about the llvm-commits
mailing list