Subzero. X86. Refactors Address Mode formation.

src/IceTargetLoweringX86Base.h

Index: src/IceTargetLoweringX86Base.h
diff --git a/src/IceTargetLoweringX86Base.h b/src/IceTargetLoweringX86Base.h
index 72e4f20fb42f5d29ef340d8a039b4a835a09c1d1..d582a0dfc8884b89f4ee16120b0427ef28460e65 100644
--- a/src/IceTargetLoweringX86Base.h
+++ b/src/IceTargetLoweringX86Base.h
@@ -192,7 +192,17 @@ protected:
 
   void postLower() override;
 
+  /// Initializes the SandboxPtr member variable -- if so required by
+  /// SandboxingType for the concrete Target.
+  void initSandboxPtr() {
+    assert(SandboxingType != ST_None);
+    dispatchToConcrete(&Traits::ConcreteTarget::initSandboxPtr);
+  }
+
+  /// Emit code that initializes the value of the SandboxPtr near the start of
+  /// the function -- if so required by SandboxingType for the concrete type.
   void initSandbox() {
+    assert(SandboxingType != ST_None);
     dispatchToConcrete(&Traits::ConcreteTarget::initSandbox);
   }
 
@@ -223,6 +233,25 @@ protected:
                                           Type ReturnType);
   uint32_t getCallStackArgumentsSizeBytes(const InstCall *Instr) override;
   void genTargetHelperCallFor(Inst *Instr) override;
+
+  /// OptAddr wraps all the possible operands that an x86 address might have.
+  struct OptAddr {

[Jim Stichnoth, 2016/01/22 02:44:10]

For better or worse, I *think* we go ahead and call it a class, not struct, when
it has methods or nontrivial ctors.

[John, 2016/01/22 02:56:21]

But this has no methods, and it has a quasi-dumb, memseting ctor. :(

[Jim Stichnoth, 2016/01/22 03:08:55]

On further thought, I take this back.  If you make it a class, do you also have
to delete default copy ctor and similar boilerplate?  Yuck.

So never mind, I like it better this way. :)

[John, 2016/01/22 04:18:07]

Not really. In theory, that should be done for structs, too, because the
compiler does generate those ctors/operators for them. It turns out that this
object is just a dumb collection of attributes, and no logic. Therefore, letting
the compiler generate those methods --if needed-- is harmless. And the compiler
will probably do a good job of memcpy() these.

by the way, I am using the copy constructor for this type. :)

+    Variable *Base = nullptr;
+    Variable *Index = nullptr;
+    uint16_t Shift = 0;
+    int32_t Offset = 0;
+    ConstantRelocatable *Relocatable = nullptr;
+  };
+  /// Legalizes Addr w.r.t. SandboxingType. The exact type of legalization
+  /// varies for different <Target, SandboxingType> tuples.
+  bool legalizeOptAddrForSandbox(OptAddr *Addr) {
+    return dispatchToConcrete(
+        &Traits::ConcreteTarget::legalizeOptAddrForSandbox, std::move(Addr));
+  }
+  // Builds information for a canonical address expresion:
+  //   <Relocatable + Offset>(Base, Index, Shift)
+  X86OperandMem *computeAddressOpt(const Inst *Instr, Type MemType,
+                                   Operand *Addr);
   void doAddressOptLoad() override;
   void doAddressOptStore() override;
   void doMockBoundsCheck(Operand *Opnd) override;
@@ -309,7 +338,8 @@ protected:
     Legal_Imm = 1 << 1,
     Legal_Mem = 1 << 2, // includes [eax+4*ecx] as well as [esp+12]
     Legal_Rematerializable = 1 << 3,
-    Legal_AddrAbs = 1 << 4, // ConstantRelocatable doesn't have to add GotVar
+    Legal_AddrAbs =
+        1 << 4, // ConstantRelocatable doesn't have to add SandboxPtr
     Legal_Default = ~(Legal_Rematerializable | Legal_AddrAbs)
     // TODO(stichnot): Figure out whether this default works for x86-64.
   };
@@ -397,11 +427,9 @@ protected:
 
     template <typename... T>
     AutoMemorySandboxer(typename Traits::TargetLowering *Target, T... Args)
-        : Target(Target),
-          MemOperand(
-              (!Traits::Is64Bit || !Target->Ctx->getFlags().getUseSandboxing())
-                  ? nullptr
-                  : findMemoryReference(Args...)) {
+        : Target(Target), MemOperand(Target->SandboxingType == ST_None
+                                         ? nullptr
+                                         : findMemoryReference(Args...)) {
       if (MemOperand != nullptr) {
         Bundler = makeUnique<AutoBundle>(Target, BundleLockOpt);
         *MemOperand = Target->_sandbox_mem_reference(*MemOperand);
@@ -914,9 +942,9 @@ protected:
       RegisterAliases;
   llvm::SmallBitVector RegsUsed;
   std::array<VarList, IceType_NUM> PhysicalRegisters;
-  // GotVar is a Variable that holds the GlobalOffsetTable address for Non-SFI
-  // mode.
-  Variable *GotVar = nullptr;
+  // SandboxPtr is a Variable that holds the GlobalOffsetTable address for

[Jim Stichnoth, 2016/01/22 02:44:10]

This comment should also reflect its use in sandboxing mode.

[John, 2016/01/22 04:18:07]

Done.

+  // Non-SFI mode.
+  Variable *SandboxPtr = nullptr;

[Jim Stichnoth, 2016/01/22 02:44:10]

I have to say, I'm not thrilled with the various uses of "Sandbox" to refer to
both sandboxing and the x86-32 GotVar for nonsfi.

Problem is, I can't think of something that's clearly better.

It would be nice if this and the "Rebased" part of x86 MemOperand could have
names that were more clearly linked.

I'll think about it some more.

[John, 2016/01/22 02:56:21]

How about RebasingPtr? RebasePtr?

 
   /// Randomize a given immediate operand
   Operand *randomizeOrPoolImmediate(Constant *Immediate,
@@ -984,10 +1012,6 @@ private:
   /// Optimizations for idiom recognition.
   bool lowerOptimizeFcmpSelect(const InstFcmp *Fcmp, const InstSelect *Select);
 
-  /// Emit code that initializes the value of the GotVar near the start of the
-  /// function.  (This code is emitted only in Non-SFI mode.)
-  void initGotVarIfNeeded();
-
   /// Complains loudly if invoked because the cpu can handle 64-bit types
   /// natively.
   template <typename T = Traits>