This is an archive of the discontinued LLVM Phabricator instance.

[x86] Rewrite getAddressFromInstr helper function
ClosedPublic

Authored by aivchenk on Sep 26 2016, 1:50 PM.

Details

Summary
  • It does not modify the input instruction
  • Second operand of any address is always an Index Register, make sure we actually check for that, instead of a check for an immediate value
  • Add two asserts along the way

Diff Detail

Repository
rL LLVM

Event Timeline

aivchenk updated this revision to Diff 72557.Sep 26 2016, 1:50 PM
aivchenk retitled this revision from to [x86] Rewrite getAddressFromInstr helper function.
aivchenk updated this object.
aivchenk updated this object.
aivchenk added a subscriber: llvm-commits.
ABataev added inline comments.
lib/Target/X86/X86InstrBuilder.h
114–118 ↗(On Diff #72557)

Do not enclose single line substatements into braces

aivchenk updated this revision to Diff 72617.Sep 27 2016, 2:17 AM

Fixed formatting

aivchenk marked an inline comment as done.Sep 30 2016, 5:57 AM
sunfish accepted this revision.Nov 21 2016, 12:23 PM
sunfish edited edge metadata.

Looks good to me. It's surprising that a function called getAddressFromInstr would mutate a MachineOperand for the input.

lib/Target/X86/X86InstrBuilder.h
107 ↗(On Diff #72617)

MachineOperand::getImm() asserts isImm(), so it's redundant to assert isImm() before calling it. Same for getReg below.

This revision is now accepted and ready to land.Nov 21 2016, 12:23 PM
This revision was automatically updated to reflect the committed changes.