[PATCH] D11665: Explicitly clear the MCInst operand list when calling the target disassembler. Otherwise, repeatedly passing in the same MCInst to getInstruction() will append the current instruction's operands to the previous instruction's operands.

Ulrich Weigand ulrich.weigand at de.ibm.com
Mon Aug 3 05:13:34 PDT 2015


uweigand added a comment.

In general, this looks good to me.  As a stylistic preference, I'd rather place the .clear() calls closer to the initial .setOpcode() calls to keep the sequences that fill in the MCInst close together; see below.


================
Comment at: lib/Target/X86/Disassembler/X86Disassembler.cpp:151
@@ -150,1 +150,3 @@
 
+  Instr.clear();
+
----------------
I think this can be pushed down into translateInstruction().

================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:2093
@@ -2092,2 +2092,3 @@
      << "  DecodeStatus S = MCDisassembler::Success;\n"
+     << "  MI.clear();\n"
      << "  for (;;) {\n"
----------------
And this can be pushed down into the MCD::OPC_Decode case.  (In the MCD::OPC_TryDecode case, we're using a temporary anyway.)


http://reviews.llvm.org/D11665







More information about the llvm-commits mailing list