[PATCH] [X86] Simplify some stuff in X86DisassemblerDecoder.

[Douglas Katzman]

- There's no reason to use a switch statement to perform left-shift.
    
- Deciding that insn->sibIndex is SIB_INDEX_NONE does not require any check beyond the fully decoded bits being equal to 0x4.
   The expression `insn->sibIndex == SIB_INDEX_sib` could not have been true unless index were 0x4, because SIB_INDEX_sib is merely the range base (SIB_INDEX_EAX) plus 4. Respectively SIB_INDEX_sib64.

Moreover, the enumerated values "SIB_INDEX_sib" and "SIB_INDEX_sib64" are misnomers, best avoided, because really they mean "no index". But the names are auto-generated, and make sense for EA_BASE_sib, EA_BASE_sib64 which mean what they say.

http://reviews.llvm.org/D9762

Files:
  lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

Index: lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
===================================================================
--- lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
+++ lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
@@ -1165,35 +1165,30 @@
     return -1;
 
   index = indexFromSIB(insn->sib) | (xFromREX(insn->rexPrefix) << 3);
+
+  // FIXME: The fifth bit (bit index 4) is only to be used for instructions
+  // that understand VSIB indexing. ORing the bit in here is mildy dangerous
+  // because performing math on an 'enum SIBIndex' can produce garbage.
+  // Excluding the "none" value, it should cover 6 spaces of register names:
+  //   - 16 possibilities for 16-bit GPR starting at SIB_INDEX_BX_SI
+  //   - 16 possibilities for 32-bit GPR starting at SIB_INDEX_EAX
+  //   - 16 possibilities for 64-bit GPR starting at SIB_INDEX_RAX
+  //   - 32 possibilities for each of XMM, YMM, ZMM registers
+  // When sibIndexBase gets assigned SIB_INDEX_RAX as it does in 64-bit mode,
+  // summing in a fully decoded index between 0 and 31 can end up with a value
+  // that looks like something in the low half of the XMM range.
+  // translateRMMemory() tries to reverse the damage, with only partial success,
+  // as evidenced by known bugs in "test/MC/Disassembler/X86/x86-64.txt"
   if (insn->vectorExtensionType == TYPE_EVEX)
     index |= v2FromEVEX4of4(insn->vectorExtensionPrefix[3]) << 4;
 
-  switch (index) {
-  case 0x4:
+  if (index == 0x4) {
     insn->sibIndex = SIB_INDEX_NONE;
-    break;
-  default:
+  } else {
     insn->sibIndex = (SIBIndex)(sibIndexBase + index);
-    if (insn->sibIndex == SIB_INDEX_sib ||
-        insn->sibIndex == SIB_INDEX_sib64)
-      insn->sibIndex = SIB_INDEX_NONE;
-    break;
   }
 
-  switch (scaleFromSIB(insn->sib)) {
-  case 0:
-    insn->sibScale = 1;
-    break;
-  case 1:
-    insn->sibScale = 2;
-    break;
-  case 2:
-    insn->sibScale = 4;
-    break;
-  case 3:
-    insn->sibScale = 8;
-    break;
-  }
+  insn->sibScale = 1 << scaleFromSIB(insn->sib);
 
   base = baseFromSIB(insn->sib) | (bFromREX(insn->rexPrefix) << 3);

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D9762.25732.patch
Type: text/x-patch
Size: 2126 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150513/c63f056b/attachment.bin>