Subzero. X8664. Enables RIP-based addressing mode.

src/IceTargetLoweringX8664.cpp

 //===- subzero/src/IceTargetLoweringX8664.cpp - x86-64 lowering -----------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 276 matching lines @@
   } else {
     _push_rbp();
   }
 }
 
 void TargetX8664::emitGetIP(CfgNode *Node) {
   // No IP base register is needed on X86-64.
   (void)Node;
 }
 
+namespace {
+bool isAssignedToRspOrRbp(const Variable *Var) {
+  if (Var == nullptr) {
+    return false;
+  }
+
+  if (Var->isRematerializable()) {
+    return true;
+  }
+
+  if (!Var->hasReg()) {

[Jim Stichnoth, 2016/01/23 01:57:47]

Optional: You could skip this condition if you want, since getRegNum() returns
NoRegister if !hasReg().

[John, 2016/01/26 19:44:25]

I did not know that. I will leave this as is -- explicit check will not leave
the reader wondering "what if !Var->hasReg()"

+    return false;
+  }
+
+  int32_t RegNum = Var->getRegNum();

[Jim Stichnoth, 2016/01/23 01:57:47]

const

[John, 2016/01/26 19:44:25]

Done.

+  if ((RegNum == Traits::RegisterSet::Reg_rsp) ||
+      (RegNum == Traits::RegisterSet::Reg_rbp)) {
+    return true;
+  }
+
+  return false;
+}
+} // end of anonymous namespace
+
 Traits::X86OperandMem *TargetX8664::_sandbox_mem_reference(X86OperandMem *Mem) {
-  // In x86_64-nacl, all memory references are relative to %r15 (i.e., %rzp.)
-  // NaCl sandboxing also requires that any registers that are not %rsp and
-  // %rbp to be 'truncated' to 32-bit before memory access.
   if (SandboxingType == ST_None) {
     return Mem;
   }
 
   if (SandboxingType == ST_Nonsfi) {
     llvm::report_fatal_error(
         "_sandbox_mem_reference not implemented for nonsfi");
   }
 
+  // In x86_64-nacl, all memory references are relative to a base register
+  // (%r15, %rsp, %rbp, or %rip).
+
   Variable *Base = Mem->getBase();
   Variable *Index = Mem->getIndex();
   uint16_t Shift = 0;
-  Variable *ZeroReg =
-      getPhysicalRegister(Traits::RegisterSet::Reg_r15, IceType_i64);
+  Variable *ZeroReg = RebasePtr;
   Constant *Offset = Mem->getOffset();
   Variable *T = nullptr;
 
+  if (Base == nullptr && Index == nullptr) {
+    // Mem is RIP-relative. There's no need to rebase it.
+    return Mem;
+  }
+
   if (Mem->getIsRebased()) {
-    // If Mem.IsRebased, then we don't need to update Mem to contain a reference
-    // to a valid base register (%r15, %rsp, or %rbp), but we still need to
-    // truncate Mem.Index (if any) to 32-bit.
-    assert(ZeroReg == Base || Base->isRematerializable());
-    T = makeReg(IceType_i32);
-    _mov(T, Index);
-    Shift = Mem->getShift();
+    // If Mem.IsRebased, then we don't need to update Mem, as it's already been
+    // updated to contain a reference to one of %rsp, %rbp, or %r15.
+    // We don't return early because we still need to zero extend Index.

[Jim Stichnoth, 2016/01/23 01:57:47]

Is the "don't return early" comment because of my derp comment in the previous
CL?  If so, don't consider it necessary here. :)

[John, 2016/01/26 19:44:25]

no, it is not. The previous comment said "but we still need to truncate
Mem.Index (if any) to 32-bit." That obscure comment really meant "we can't
return early because with need to movzx(Index)."

+    assert(ZeroReg == Base || isAssignedToRspOrRbp(Base));
+    ZeroReg = Base;
+    if (Index != nullptr) {
+      T = makeReg(IceType_i32);
+      _mov(T, Index);
+      Shift = Mem->getShift();
+    }
   } else {
     if (Base != nullptr) {
-      if (Base->isRematerializable()) {
+      // If Base is a valid base pointer we don't need to use the RebasePtr. By
+      // doing this we might save us the need to zero extend the memory operand.
+      if (isAssignedToRspOrRbp(Base)) {
         ZeroReg = Base;
       } else {
         T = Base;
       }
     }
 
     if (Index != nullptr) {
       assert(!Index->isRematerializable());
+      // If Index is not nullptr, it is mandatory that T is a nullptr.
+      // Otherwise, the lowering generated an memory operand with two registers.

[Jim Stichnoth, 2016/01/23 01:57:47]

s/an/a/

[John, 2016/01/26 19:44:25]

Done.

+      // Note that Base might still be non-nullptr, but it must be a valid
+      // base register.
       if (T != nullptr) {
         llvm::report_fatal_error("memory reference contains base and index.");
       }
-      T = Index;
-      Shift = Mem->getShift();
+      // If the Index is not shifted, and it is a Valid Base, and the ZeroReg is
+      // still RebasePtr, then we do ZeroReg = Index, and hopefully prevent the
+      // need to zero-extend the memory operand (which may still happen -- see
+      // NeedLea below.)
+      if (Shift == 0 && isAssignedToRspOrRbp(Index) && ZeroReg == RebasePtr) {
+        ZeroReg = Index;
+      } else {
+        T = Index;
+        Shift = Mem->getShift();
+      }
     }
   }
 
   // NeedsLea is a flags indicating whether Mem needs to be materialized to a
   // GPR prior to being used. A LEA is needed if Mem.Offset is a constant
   // relocatable, or if Mem.Offset is negative. In both these cases, the LEA is
   // needed to ensure the sandboxed memory operand will only use the lower
   // 32-bits of T+Offset.
   bool NeedsLea = false;
   if (const auto *Offset = Mem->getOffset()) {
     if (llvm::isa<ConstantRelocatable>(Offset)) {
       NeedsLea = true;
     } else if (const auto *Imm = llvm::cast<ConstantInteger32>(Offset)) {
       NeedsLea = Imm->getValue() < 0;
     }
   }
 
   int32_t RegNum = Variable::NoRegister;
   int32_t RegNum32 = Variable::NoRegister;
   if (T != nullptr) {
     if (T->hasReg()) {
       RegNum = Traits::getGprForType(IceType_i64, T->getRegNum());
       RegNum32 = Traits::getGprForType(IceType_i32, RegNum);
-      switch (RegNum) {
-      case Traits::RegisterSet::Reg_rsp:
-      case Traits::RegisterSet::Reg_rbp:
-        // Memory operands referencing rsp/rbp do not need to be sandboxed.
-        return Mem;
-      }
+      // At this point, if T was assigned to rsp/rbp, then we would have already
+      // made this the ZeroReg.
+      assert(RegNum != Traits::RegisterSet::Reg_rsp);
+      assert(RegNum != Traits::RegisterSet::Reg_rbp);
     }
 
     switch (T->getType()) {
     default:
+      llvm::report_fatal_error("Mem pointer should be a 32-bit integer.");

[Jim Stichnoth, 2016/01/23 01:57:47]

Here and below, would it make more sense to say "GPR" instead of "integer"?

[John, 2016/01/26 19:44:25]

I tend to think about GPRs/Regs in the assembler. In the lowering, I believe it
makes more sense to think about temporaries, integer types (which will
eventually be mapped to 32-bit registers, maybe) and other higher-level
entities. but no strong opinions, so done.

     case IceType_i64:
       // Even though "default:" would also catch T.Type == IceType_i64, an
       // explicit 'case IceType_i64' shows that memory operands are always
       // supposed to be 32-bits.
-      llvm::report_fatal_error("Mem pointer should be 32-bit.");
+      llvm::report_fatal_error("Mem pointer should not be a 64-bit integer.");
     case IceType_i32: {
       Variable *T64 = makeReg(IceType_i64, RegNum);
       auto *Movzx = _movzx(T64, T);
       if (!NeedsLea) {
         // This movzx is only needed when Mem does not need to be lea'd into a
         // temporary. If an lea is going to be emitted, then eliding this movzx
         // is safe because the emitted lea will write a 32-bit result --
         // implicitly zero-extended to 64-bit.
         Movzx->setMustKeep();
       }
@@ skipping 326 matching lines @@
 #define X(tag, sizeLog2, align, elts, elty, str)                               \
   static_assert(_table1_##tag == _table2_##tag,                                \
                 "Inconsistency between ICETYPEX8664_TABLE and ICETYPE_TABLE");
 ICETYPE_TABLE
 #undef X
 } // end of namespace dummy3
 } // end of anonymous namespace
 
 } // end of namespace X8664
 } // end of namespace Ice