This is an archive of the discontinued LLVM Phabricator instance.

[X86] Simplify some stuff in X86DisassemblerDecoder.
ClosedPublic

Authored by dougk on May 13 2015, 2:10 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

dougk updated this revision to Diff 25732.May 13 2015, 2:10 PM
dougk retitled this revision from to [X86] Simplify some stuff in X86DisassemblerDecoder..
dougk updated this object.
dougk edited the test plan for this revision. (Show Details)
dougk added a subscriber: Unknown Object (MLST).
chandlerc added a subscriber: 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.

dougk accepted this revision.Jun 24 2015, 2:36 PM
dougk added a reviewer: dougk.

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

This revision is now accepted and ready to land.Jun 24 2015, 2:36 PM
This revision was automatically updated to reflect the committed changes.