[x86] Rewrite getAddressFromInstr helper function

[aivchenk]

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

[ABataev]

lib/Target/X86/X86InstrBuilder.h

114–118

Do not enclose single line substatements into braces

[aivchenk]

Fixed formatting

[sunfish]

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

lib/Target/X86/X86InstrBuilder.h

122

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

lib/Target/X86/X86InstrBuilder.h

void getFullAddress(SmallVectorImpl<MachineOperand> &MO) {
 
     MO.push_back(MachineOperand::CreateReg(0, false, false, false, false, false,
                                            false, 0, false));
   }
 };
 
 /// Compute the addressing mode from an machine instruction starting with the
 /// given operand.
-static inline X86AddressMode getAddressFromInstr(MachineInstr *MI,
+static inline X86AddressMode getAddressFromInstr(const MachineInstr *MI,
                                                  unsigned Operand) {
   X86AddressMode AM;
-  MachineOperand &Op = MI->getOperand(Operand);
-  if (Op.isReg()) {
+  const MachineOperand &Op0 = MI->getOperand(Operand);
+  if (Op0.isReg()) {
     AM.BaseType = X86AddressMode::RegBase;
-    AM.Base.Reg = Op.getReg();
+    AM.Base.Reg = Op0.getReg();
   } else {
     AM.BaseType = X86AddressMode::FrameIndexBase;
-    AM.Base.FrameIndex = Op.getIndex();
+    AM.Base.FrameIndex = Op0.getIndex();
   }
-  Op = MI->getOperand(Operand + 1);
-  if (Op.isImm())
-    AM.Scale = Op.getImm();
-  Op = MI->getOperand(Operand + 2);
-  if (Op.isImm())
-    AM.IndexReg = Op.getImm();
-  Op = MI->getOperand(Operand + 3);
-  if (Op.isGlobal()) {
-    AM.GV = Op.getGlobal();
+
+  const MachineOperand &Op1 = MI->getOperand(Operand + 1);
+  assert(Op1.isImm() && "Second address operand must be an immediate value");
+  AM.Scale = Op1.getImm();
+
+  const MachineOperand &Op2 = MI->getOperand(Operand + 2);
+  assert(Op2.isReg() && "Third address operand must be a register");
+  AM.IndexReg = Op2.getReg();
+
+  const MachineOperand &Op3 = MI->getOperand(Operand + 3);
+  if (Op3.isGlobal()) {
+    AM.GV = Op3.getGlobal();
   } else {
-    AM.Disp = Op.getImm();
+    AM.Disp = Op3.getImm();
   }
   return AM;
 }
 
 /// addDirectMem - This function is used to add a direct memory reference to the
 /// current instruction -- that is, a dereference of an address in a register,
 /// with no scale, index or displacement. An example is: DWORD PTR [EAX].
 ///
 static inline const MachineInstrBuilder &
 addDirectMem(const MachineInstrBuilder &MIB, unsigned Reg) {
   // Because memory references are always represented with five
   // values, this adds: Reg, 1, NoReg, 0, NoReg to the instruction.
   return MIB.addReg(Reg).addImm(1).addReg(0).addImm(0).addReg(0);