[llvm] Handle MCInst op, non-null MCInst op, tgt MCExpr; fix AddressMap (PR #192188)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Brian Cain (androm3da)

<details>
<summary>Changes</summary>

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.

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


4 Files Affected:

- (modified) bolt/include/bolt/Core/MCPlus.h (+8-2) 
- (modified) bolt/include/bolt/Core/MCPlusBuilder.h (+1-3) 
- (modified) bolt/lib/Core/AddressMap.cpp (+4-3) 
- (modified) bolt/lib/Core/MCPlusBuilder.cpp (+7-1) 


``````````diff
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();
   }
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)) {
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!");

``````````

</details>


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