[PATCH] D126101: [MCDisassembler] Fix MCSymbolizer::tryAddingSymbolicOperand() interface

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/unittests/MC/X86/CMakeLists.txt:10
+  Target
+  X86Desc
+  X86Disassembler
----------------
Add X86Info

otherwise -DBUILD_SHARED_LIBS=on build will have a link error.


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:25
+struct Context {
+  const char *TripleName = "x86_64-pc-linux";
+  std::unique_ptr<MCRegisterInfo> MRI;
----------------
or use x86_64-unknown-elf


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:62
+
+class X86MCSymbolizerTest : public MCSymbolizer {
+public:
----------------
Place X86MCSymbolizerTest in an anonymous namespace, too.


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:94
+TEST(X86Disassembler, X86MCSymbolizerTest) {
+  if (!getContext())
+    return;
----------------
Delete


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:121
+
+  // movq   $0xffffffffffffefe8,-0x1(%rip)
+  // Test that the value of the rip-relative operand is set correctly.
----------------
Consider testing something more complex, like `291(%rax,%r14,8)`


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:127
+  checkOperand(0, /*next instr address*/ 11 - /*disp*/ 1, 3, 4);
+  checkOperand(1, 0xffffffffffffefe8, 7, 4);
+
----------------
Additionally test `InstSize`


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:136
+
+  // movq    $0x80000, 0x80000
+  checkBytes(
----------------
Suggest reordering the instructions from simple to complex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126101