[PATCH] D121065: [X86] Fix MCSymbolizer interface for X86Disassembler

Kan Shengchen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 6 17:50:47 PST 2022


skan added inline comments.


================
Comment at: llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp:1992-1993
 
-  if(!tryAddingSymbolicOperand(immediate + pcrel, isBranch, insn.startLocation,
-                               insn.immediateOffset, insn.immediateSize,
-                               mcInst, Dis))
+  if (!tryAddingSymbolicOperand(immediate + pcrel, isBranch, insn.startLocation,
+                                insn.immediateOffset, insn.length, mcInst, Dis))
     mcInst.addOperand(MCOperand::createImm(immediate));
----------------
I am confused what you are fixing here. But it's not a clean fix at least, the comments of the function is not updated. Could you provide a LIT test?


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:79-85
+  bool tryAddingSymbolicOperand(MCInst &Inst, raw_ostream &CStream,
+                                int64_t Value, uint64_t Address, bool IsBranch,
+                                uint64_t Offset, uint64_t InstSize) override {
+    Operands.push_back({Value, Offset});
+    InstructionSize = InstSize;
+    return false;
+  }
----------------
Why redefine the function here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121065/new/

https://reviews.llvm.org/D121065



More information about the llvm-commits mailing list