[llvm] Handle MCInst op, non-null MCInst op, tgt MCExpr; fix AddressMap (PR #192188)
Brian Cain via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 14 22:47:33 PDT 2026
https://github.com/androm3da created https://github.com/llvm/llvm-project/pull/192188
AddressMap: Fix the parse side to use a hardcoded width of 8 to match emit.
Add support for comparing MCInst operands in MCPlusBuilder::equals() and handle MCExpr::Target expressions by returning false instead of hitting llvm_unreachable. This is needed for targets like Hexagon where the disassembler produces MCInst operands (sub-instructions inside duplex bundles) and target-specific MCExpr types (HexagonMCExpr).
Support non-null MCInst operands in annotation handling: The annotation sentinel in BOLT is a null MCInst operand appended after all prime operands. However, some architectures (e.g. Hexagon) use non-null MCInst operands as legitimate prime operands for duplex sub-instructions.
>From a08e0e619e8cd8e85cb5ee512b51f076131f8de6 Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Tue, 7 Apr 2026 20:21:15 -0700
Subject: [PATCH 1/3] [BOLT] Support non-null MCInst operands in annotation
handling
The annotation sentinel in BOLT is a null MCInst operand appended
after all prime operands. However, some architectures (e.g. Hexagon)
use non-null MCInst operands as legitimate prime operands for duplex
sub-instructions. The existing code treated any MCInst operand as
the annotation sentinel, causing duplex sub-instructions to be
misidentified.
In getNumPrimeOperands(), only treat a null MCInst operand as the
sentinel. In getAnnotationInstOp(), skip non-null MCInst operands
when searching for the annotation sentinel.
---
bolt/include/bolt/Core/MCPlus.h | 10 ++++++++--
bolt/include/bolt/Core/MCPlusBuilder.h | 4 +---
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlus.h b/bolt/include/bolt/Core/MCPlus.h
index ead6ba1470da6..382e21606d487 100644
--- a/bolt/include/bolt/Core/MCPlus.h
+++ b/bolt/include/bolt/Core/MCPlus.h
@@ -117,8 +117,14 @@ template <typename ValueType> class MCSimpleAnnotation : public MCAnnotation {
/// annotations.
inline unsigned getNumPrimeOperands(const MCInst &Inst) {
for (signed I = Inst.getNumOperands() - 1; I >= 0; --I) {
- if (Inst.getOperand(I).isInst())
- return I;
+ if (Inst.getOperand(I).isInst()) {
+ // Only a null MCInst operand is the annotation sentinel.
+ // Non-null MCInst operands (e.g. Hexagon duplex sub-instructions)
+ // are legitimate prime operands.
+ if (Inst.getOperand(I).getInst() == nullptr)
+ return I;
+ return Inst.getNumOperands();
+ }
if (!Inst.getOperand(I).isImm())
return Inst.getNumOperands();
}
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index 0f6076688b65d..85fe6fff3a878 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -138,10 +138,8 @@ class MCPlusBuilder {
MCInst::iterator getAnnotationInstOp(MCInst &Inst) const {
for (MCInst::iterator Iter = Inst.begin(); Iter != Inst.end(); ++Iter) {
- if (Iter->isInst()) {
- assert(Iter->getInst() == nullptr && "Empty instruction expected.");
+ if (Iter->isInst() && Iter->getInst() == nullptr)
return Iter;
- }
}
return Inst.end();
}
>From 31c245d08d8577eeeaf84def1cf7e942ced984c4 Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Tue, 14 Apr 2026 22:24:36 -0700
Subject: [PATCH 2/3] [BOLT] Handle MCInst operands and target MCExpr in
MCPlusBuilder
Add support for comparing MCInst operands in MCPlusBuilder::equals()
and handle MCExpr::Target expressions by returning false instead of
hitting llvm_unreachable.
This is needed for targets like Hexagon where the disassembler
produces MCInst operands (sub-instructions inside duplex bundles)
and target-specific MCExpr types (HexagonMCExpr).
Without this, BOLT would crash when encountering these operand
or expression kinds during instruction comparison.
---
bolt/lib/Core/MCPlusBuilder.cpp | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 0cb4ba1ebfbd7..45a12110e2bd1 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -79,6 +79,10 @@ bool MCPlusBuilder::equals(const MCOperand &A, const MCOperand &B,
if (!B.isExpr())
return false;
return equals(*A.getExpr(), *B.getExpr(), Comp);
+ } else if (A.isInst()) {
+ if (!B.isInst())
+ return false;
+ return equals(*A.getInst(), *B.getInst(), Comp);
} else {
llvm_unreachable("unexpected operand kind");
return false;
@@ -125,7 +129,9 @@ bool MCPlusBuilder::equals(const MCExpr &A, const MCExpr &B,
return equals(TargetExprA, TargetExprB, Comp);
}
case MCExpr::Target:
- llvm_unreachable("Not implemented");
+ // Target-specific expressions (e.g. HexagonMCExpr) cannot be compared
+ // without target-specific knowledge. Conservatively return false.
+ return false;
}
llvm_unreachable("Invalid expression kind!");
>From 6f3925e0b0e128fabe32fece351d89f6709b89b1 Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Tue, 14 Apr 2026 22:25:06 -0700
Subject: [PATCH 3/3] [BOLT] Fix AddressMap parse to match emit side width
The emit side of AddressMap uses emitIntValue(..., 8) and
emitSymbolValue(..., 8) unconditionally, but the parse side was
using BC.AsmInfo->getCodePointerSize() which could be 4 on 32-bit
targets. This mismatch would cause incorrect parsing of the
address map on non-64-bit targets.
Fix the parse side to use a hardcoded width of 8 to match emit.
Both input/output addresses and label pointers are stored as 8
bytes: BOLT uses uint64_t addresses internally, and the label
map stores host pointers (8 bytes on 64-bit hosts) cast to
integers.
---
bolt/lib/Core/AddressMap.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/bolt/lib/Core/AddressMap.cpp b/bolt/lib/Core/AddressMap.cpp
index efa376d408db8..b9de21f81b362 100644
--- a/bolt/lib/Core/AddressMap.cpp
+++ b/bolt/lib/Core/AddressMap.cpp
@@ -70,15 +70,16 @@ std::optional<AddressMap> AddressMap::parse(BinaryContext &BC) {
AddressMap Parsed;
- const size_t EntrySize = 2 * BC.AsmInfo->getCodePointerSize();
+ // Entries are always 2x8 bytes: the emit side uses emitIntValue(..., 8)
+ // and emitSymbolValue(..., 8) regardless of target pointer size.
+ const size_t EntrySize = 2 * 8;
auto parseSection =
[&](BinarySection &Section,
function_ref<void(uint64_t, uint64_t)> InsertCallback) {
StringRef Buffer = Section.getOutputContents();
assert(Buffer.size() % EntrySize == 0 && "Unexpected address map size");
- DataExtractor DE(Buffer, BC.AsmInfo->isLittleEndian(),
- BC.AsmInfo->getCodePointerSize());
+ DataExtractor DE(Buffer, BC.AsmInfo->isLittleEndian(), 8);
DataExtractor::Cursor Cursor(0);
while (Cursor && !DE.eof(Cursor)) {
More information about the llvm-commits
mailing list