[llvm] [BOLT] Do not return Def-ed registers from MCPlusBuilder::getUsedRegs (PR #129890)
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 21 09:04:32 PDT 2025
https://github.com/atrosinenko updated https://github.com/llvm/llvm-project/pull/129890
>From b0307056b0a1c05c8d50a5691c13e114adc5a1b7 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 a3be147a09066..7752079b61538 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 a3113cab3d334..1d5dc00742a61 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) {
@@ -155,6 +171,100 @@ TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) {
ASSERT_EQ(Label, BB->getLabel());
}
+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 62178dfada1029f4281e27b41acf9b139bfd2c00 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 1d5dc00742a61..7016dec0e3574 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, AArch64_CmpJE) {
@@ -273,15 +269,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