[X86] Simplify some stuff in X86DisassemblerDecoder.

[dougk]

Summary

• 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.

[chandlerc]

While I *think* this is fine, at this point I'd really rather Craig chime in. He is probably the most familiar with this area of the x86 backend.

[craig.topper]

LGTM

[dougk]

looks like Craig forgot to press 'Accept' so I will

[craig.topper]

Oops. Sorry.

llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

case 8:
     sibBaseBase = SIB_BASE_RAX;
     break;
   }
 
   if (consumeByte(insn, &insn->sib))
     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);
 
   switch (base) {
   case 0x5:
   case 0xd:
     switch (modFromModRM(insn->modRM)) {
     case 0x0: