Enhance address mode recovery

src/IceTargetLoweringX8632Traits.h

 //===- subzero/src/IceTargetLoweringX8632Traits.h - x86-32 traits -*- C++ -*-=//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 164 matching lines @@
     Address() = delete;
 
   public:
     Address(const Address &other) : Operand(other) {}
 
     Address &operator=(const Address &other) {
       Operand::operator=(other);
       return *this;
     }
 
-    Address(GPRRegister base, int32_t disp) {
+    Address(GPRRegister base, int32_t disp, AssemblerFixup *fixup = nullptr) {

[Jim Stichnoth, 2015/10/27 05:52:34]

Maybe we can make fixup a required parameter, rather than default-value?

[sehr, 2015/10/27 21:48:00]

Done.

       if (disp == 0 && base != RegX8632::Encoded_Reg_ebp) {
         SetModRM(0, base);
         if (base == RegX8632::Encoded_Reg_esp)
           SetSIB(TIMES_1, RegX8632::Encoded_Reg_esp, base);
       } else if (Utils::IsInt(8, disp)) {
         SetModRM(1, base);
         if (base == RegX8632::Encoded_Reg_esp)
           SetSIB(TIMES_1, RegX8632::Encoded_Reg_esp, base);
         SetDisp8(disp);
+        // TODO(sehr): determine if there are 8-bit relocations.

[Jim Stichnoth, 2015/10/27 13:57:10]

Here and in the TODO below, if there is a fixup, I think you need to just use a
32-bit displacement.  For example, here's a mashup of simple_loop.ll and
globalrelocs.ll:

@bytes = internal global [8 x i8] c"abcdefgh"
define internal i32 @simple_loop(i32 %a, i32 %n) {
entry:
  %cmp4 = icmp sgt i32 %n, 0
  br i1 %cmp4, label %for.body, label %for.end
for.body:
  %i.06 = phi i32 [ %inc, %for.body ], [ 0, %entry ]
  %sum.05 = phi i32 [ %add, %for.body ], [ 0, %entry ]
  %gep_array = mul i32 %i.06, 4
  %gep = add i32 %a, %gep_array
  %baddr = ptrtoint [8 x i8] * @bytes to i32
  %gep2 = add i32 %gep, %baddr
  %gep3 = add i32 %gep2, 4
  %__9 = inttoptr i32 %gep3 to i32*
  %v0 = load i32, i32* %__9, align 1
  %add = add i32 %v0, %sum.05
  %inc = add i32 %i.06, 1
  %cmp = icmp slt i32 %inc, %n
  br i1 %cmp, label %for.body, label %for.end
for.end:
  %sum.0.lcssa = phi i32 [ 0, %entry ], [ %add, %for.body ]
  ret i32 %sum.0.lcssa
}

Now, change %gep3 to add 1024 instead of 4, and it works correctly.

More generally, the first two branches of this if/then/else chain should not
fire when fixup!=nullptr, because disp is really not an 8-bit 0.

[Jim Stichnoth, 2015/10/27 13:57:10]

One separate note on these fixups.  It seems that when we neglect to invoke
SetFixup on a fixup, it results in the fixup being emitted at offset 0 of the
function.  Could you fix up the fixups so that we get an assert or something
when this happens?

[sehr, 2015/10/27 21:48:00]

Agreed, and that fixed my other bug.  Thanks!

[sehr, 2015/10/27 21:48:00]

Added flag/assertions in IceFixups.h

       } else {
         SetModRM(2, base);
         if (base == RegX8632::Encoded_Reg_esp)
           SetSIB(TIMES_1, RegX8632::Encoded_Reg_esp, base);
         SetDisp32(disp);
+        if (fixup)
+          SetFixup(fixup);
       }
     }
 
-    Address(GPRRegister index, ScaleFactor scale, int32_t disp) {
+    Address(GPRRegister index, ScaleFactor scale, int32_t disp,
+            AssemblerFixup *fixup = nullptr) {
       assert(index != RegX8632::Encoded_Reg_esp); // Illegal addressing mode.
       SetModRM(0, RegX8632::Encoded_Reg_esp);
       SetSIB(scale, index, RegX8632::Encoded_Reg_ebp);
       SetDisp32(disp);
+      if (fixup)
+        SetFixup(fixup);
     }
 
     Address(GPRRegister base, GPRRegister index, ScaleFactor scale,
-            int32_t disp) {
+            int32_t disp, AssemblerFixup *fixup = nullptr) {
       assert(index != RegX8632::Encoded_Reg_esp); // Illegal addressing mode.
       if (disp == 0 && base != RegX8632::Encoded_Reg_ebp) {
         SetModRM(0, RegX8632::Encoded_Reg_esp);
         SetSIB(scale, index, base);
       } else if (Utils::IsInt(8, disp)) {
         SetModRM(1, RegX8632::Encoded_Reg_esp);
         SetSIB(scale, index, base);
         SetDisp8(disp);
+        // TODO(sehr): determine if there are 8-bit relocations.
       } else {
         SetModRM(2, RegX8632::Encoded_Reg_esp);
         SetSIB(scale, index, base);
         SetDisp32(disp);
+        if (fixup)
+          SetFixup(fixup);
       }
     }
 
     /// AbsoluteTag is a special tag used by clients to create an absolute
     /// Address.
     enum AbsoluteTag { ABSOLUTE };
 
     Address(AbsoluteTag, const uintptr_t Addr) {
       SetModRM(0, RegX8632::Encoded_Reg_ebp);
       SetDisp32(Addr);
@@ skipping 507 matching lines @@
 
 } // end of namespace X86Internal
 
 namespace X8632 {
 using Traits = ::Ice::X86Internal::MachineTraits<TargetX8632>;
 } // end of namespace X8632
 
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICETARGETLOWERINGX8632TRAITS_H