Subzero. ARM32. Removing memory legalization warts.

src/IceTargetLoweringARM32.cpp

 //===- subzero/src/IceTargetLoweringARM32.cpp - ARM32 lowering ------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 457 matching lines @@
         "Infinite-weight Variable has no register assigned");
   }
   int32_t Offset = Var->getStackOffset();
   int32_t BaseRegNum = Var->getBaseRegNum();
   if (BaseRegNum == Variable::NoRegister) {
     BaseRegNum = getFrameOrStackReg();
     if (!hasFramePointer())
       Offset += getStackAdjustment();
   }
   const Type VarTy = Var->getType();
+  /*

[Karl, 2015/11/18 18:40:42]

Remove comment?

[John, 2015/11/19 13:41:22]

Oops, done.

   if (!isLegalMemOffset(VarTy, Offset)) {
-    llvm::report_fatal_error("Illegal stack offset");
+    // llvm::report_fatal_error("Illegal stack offset");
   }
+  */
   Str << "[" << getRegName(BaseRegNum, VarTy);
   if (Offset != 0) {
     Str << ", " << getConstantPrefix() << Offset;
   }
   Str << "]";
 }
 
 bool TargetARM32::CallingConv::I64InRegs(std::pair<int32_t, int32_t> *Regs) {
   if (NumGPRRegsUsed >= ARM32_MAX_GPR_ARG)
     return false;
@@ skipping 127 matching lines @@
 // Helper function for addProlog().
 //
 // This assumes Arg is an argument passed on the stack. This sets the frame
 // offset for Arg and updates InArgsSizeBytes according to Arg's width. For an
 // I64 arg that has been split into Lo and Hi components, it calls itself
 // recursively on the components, taking care to handle Lo first because of the
 // little-endian architecture. Lastly, this function generates an instruction
 // to copy Arg into its assigned register if applicable.
 void TargetARM32::finishArgumentLowering(Variable *Arg, Variable *FramePtr,
                                          size_t BasicFrameOffset,
-                                         size_t &InArgsSizeBytes) {
+                                         size_t *InArgsSizeBytes) {
   if (auto *Arg64On32 = llvm::dyn_cast<Variable64On32>(Arg)) {
     Variable *Lo = Arg64On32->getLo();
     Variable *Hi = Arg64On32->getHi();
     finishArgumentLowering(Lo, FramePtr, BasicFrameOffset, InArgsSizeBytes);
     finishArgumentLowering(Hi, FramePtr, BasicFrameOffset, InArgsSizeBytes);
     return;
   }
   Type Ty = Arg->getType();
-  InArgsSizeBytes = applyStackAlignmentTy(InArgsSizeBytes, Ty);
-  Arg->setStackOffset(BasicFrameOffset + InArgsSizeBytes);
-  InArgsSizeBytes += typeWidthInBytesOnStack(Ty);
-  // If the argument variable has been assigned a register, we need to load the
+  assert(Ty != IceType_i64);
+
+  *InArgsSizeBytes = applyStackAlignmentTy(*InArgsSizeBytes, Ty);
+  const int32_t ArgStackOffset = BasicFrameOffset + *InArgsSizeBytes;
+  *InArgsSizeBytes += typeWidthInBytesOnStack(Ty);
+
+  if (!Arg->hasReg()) {
+    Arg->setStackOffset(ArgStackOffset);
+    return;
+  }
+
+  // If the argument variable has been assigned a register, we need to copy the
   // value from the stack slot.
-  if (Arg->hasReg()) {
-    assert(Ty != IceType_i64);
-    // This should be simple, just load the parameter off the stack using a nice
-    // sp + imm addressing mode. Because ARM, we can't do that (e.g., VLDR, for
-    // fp types, cannot have an index register), so we legalize the memory
-    // operand instead.
-    auto *Mem = OperandARM32Mem::create(
-        Func, Ty, FramePtr, llvm::cast<ConstantInteger32>(
-                                Ctx->getConstantInt32(Arg->getStackOffset())));
-    _mov(Arg, legalizeToReg(Mem, Arg->getRegNum()));
-    // This argument-copying instruction uses an explicit OperandARM32Mem
-    // operand instead of a Variable, so its fill-from-stack operation has to
-    // be tracked separately for statistics.
-    Ctx->statsUpdateFills();
-  }
+  Variable *Parameter = Func->makeVariable(Ty);
+  Parameter->setMustNotHaveReg();
+  Parameter->setStackOffset(ArgStackOffset);
+  _mov(Arg, Parameter);
 }
 
 Type TargetARM32::stackSlotType() { return IceType_i32; }
 
 void TargetARM32::addProlog(CfgNode *Node) {
   // Stack frame layout:
   //
   // +------------------------+
   // | 1. preserved registers |
   // +------------------------+
@@ skipping 80 matching lines @@
     CalleeSaves[RegARM32::Reg_lr] = true;
     RegsUsed[RegARM32::Reg_lr] = true;
   }
   for (SizeT i = 0; i < CalleeSaves.size(); ++i) {
     if (RegARM32::isI64RegisterPair(i)) {
       // We don't save register pairs explicitly. Instead, we rely on the code
       // fake-defing/fake-using each register in the pair.
       continue;
     }
     if (CalleeSaves[i] && RegsUsed[i]) {
-      // TODO(jvoung): do separate vpush for each floating point register
-      // segment and += 4, or 8 depending on type.
       ++NumCallee;
       Variable *PhysicalRegister = getPhysicalRegister(i);
       PreservedRegsSizeBytes +=
           typeWidthInBytesOnStack(PhysicalRegister->getType());
       GPRsToPreserve.push_back(getPhysicalRegister(i));
     }
   }
   Ctx->statsUpdateRegistersSaved(NumCallee);
   if (!GPRsToPreserve.empty())
     _push(GPRsToPreserve);
@@ skipping 61 matching lines @@
       InRegs = CC.FPInReg(Ty, &DummyReg);
     } else if (Ty == IceType_i64) {
       std::pair<int32_t, int32_t> DummyRegs;
       InRegs = CC.I64InRegs(&DummyRegs);
     } else {
       assert(Ty == IceType_i32);
       int32_t DummyReg;
       InRegs = CC.I32InReg(&DummyReg);
     }
     if (!InRegs)
-      finishArgumentLowering(Arg, FramePtr, BasicFrameOffset, InArgsSizeBytes);
+      finishArgumentLowering(Arg, FramePtr, BasicFrameOffset, &InArgsSizeBytes);
   }
 
   // Fill in stack offsets for locals.
   assignVarStackSlots(SortedSpilledVariables, SpillAreaPaddingBytes,
                       SpillAreaSizeBytes, GlobalsAndSubsequentPaddingSize,
                       UsesFramePointer);
   this->HasComputedFrame = true;
 
   if (BuildDefs::dump() && Func->isVerbose(IceV_Frame)) {
     OstreamLocker _(Func->getContext());
@@ skipping 123 matching lines @@
   Operand *OffsetVal = legalize(Ctx->getConstantInt32(Offset),
                                 Legal_Reg | Legal_Flex, getReservedTmpReg());
   Variable *ScratchReg = makeReg(IceType_i32, getReservedTmpReg());
   if (ShouldSub)
     _sub(ScratchReg, OrigBaseReg, OffsetVal);
   else
     _add(ScratchReg, OrigBaseReg, OffsetVal);
   return ScratchReg;
 }
 
-StackVariable *TargetARM32::legalizeStackSlot(Type Ty, int32_t Offset,
-                                              int32_t StackAdjust,
-                                              Variable *OrigBaseReg,
-                                              Variable **NewBaseReg,
-                                              int32_t *NewBaseOffset) {
+OperandARM32Mem *TargetARM32::createMemOperand(Type Ty, int32_t Offset,
+                                               int32_t StackAdjust,
+                                               Variable *OrigBaseReg,
+                                               Variable **NewBaseReg,
+                                               int32_t *NewBaseOffset) {
+  if (isLegalMemOffset(Ty, Offset + StackAdjust)) {
+    return OperandARM32Mem::create(
+        Func, Ty, OrigBaseReg, llvm::cast<ConstantInteger32>(
+                                   Ctx->getConstantInt32(Offset + StackAdjust)),
+        OperandARM32Mem::Offset);
+  }
+
   if (*NewBaseReg == nullptr) {
     *NewBaseReg = newBaseRegister(Offset, StackAdjust, OrigBaseReg);
     *NewBaseOffset = Offset + StackAdjust;
   }
 
   int32_t OffsetDiff = Offset + StackAdjust - *NewBaseOffset;
   if (!isLegalMemOffset(Ty, OffsetDiff)) {
     *NewBaseReg = newBaseRegister(Offset, StackAdjust, OrigBaseReg);
     *NewBaseOffset = Offset + StackAdjust;
     OffsetDiff = 0;
   }
 
-  StackVariable *NewDest = Func->makeVariable<StackVariable>(Ty);
-  NewDest->setMustNotHaveReg();
-  NewDest->setBaseRegNum((*NewBaseReg)->getRegNum());
-  NewDest->setStackOffset(OffsetDiff);
-  return NewDest;
+  return OperandARM32Mem::create(
+      Func, Ty, *NewBaseReg,
+      llvm::cast<ConstantInteger32>(Ctx->getConstantInt32(OffsetDiff)),
+      OperandARM32Mem::Offset);
 }
 
-void TargetARM32::legalizeMovStackAddrImm(InstARM32Mov *MovInstr,
-                                          int32_t StackAdjust,
-                                          Variable *OrigBaseReg,
-                                          Variable **NewBaseReg,
-                                          int32_t *NewBaseOffset) {
+void TargetARM32::legalizeMov(InstARM32Mov *MovInstr, int32_t StackAdjust,
+                              Variable *OrigBaseReg, Variable **NewBaseReg,
+                              int32_t *NewBaseOffset) {
   Variable *Dest = MovInstr->getDest();
   assert(Dest != nullptr);
   Type DestTy = Dest->getType();
   assert(DestTy != IceType_i64);
 
   Operand *Src = MovInstr->getSrc(0);
   Type SrcTy = Src->getType();
+  (void)SrcTy;
   assert(SrcTy != IceType_i64);
 
   if (MovInstr->isMultiDest() || MovInstr->isMultiSource())
     return;
 
   bool Legalized = false;
   if (!Dest->hasReg()) {
-    assert(llvm::cast<Variable>(Src)->hasReg());
+    auto *SrcR = llvm::cast<Variable>(Src);

[Karl, 2015/11/18 18:40:42]

const auto?

[John, 2015/11/19 13:41:22]

done.

+    assert(SrcR->hasReg());
     const int32_t Offset = Dest->getStackOffset();
-    if (!isLegalMemOffset(DestTy, Offset + StackAdjust)) {
-      Legalized = true;
-      Dest = legalizeStackSlot(DestTy, Offset, StackAdjust, OrigBaseReg,
-                               NewBaseReg, NewBaseOffset);
-    }
+    // This is a _mov(Mem(), Variable), i.e., a store.
+    _str(SrcR, createMemOperand(DestTy, Offset, StackAdjust, OrigBaseReg,
+                                NewBaseReg, NewBaseOffset),
+         MovInstr->getPredicate());
+    // _str() does not have a Dest, so we add a fake-def(Dest).
+    Context.insert(InstFakeDef::create(Func, Dest));
+    Legalized = true;
   } else if (auto *Var = llvm::dyn_cast<Variable>(Src)) {
     if (!Var->hasReg()) {
       const int32_t Offset = Var->getStackOffset();
-      if (!isLegalMemOffset(SrcTy, Offset + StackAdjust)) {
-        Legalized = true;
-        Src = legalizeStackSlot(SrcTy, Offset, StackAdjust, OrigBaseReg,
-                                NewBaseReg, NewBaseOffset);
-      }
-    }
-  } else if (auto *Mem = llvm::dyn_cast<OperandARM32Mem>(Src)) {
-    if (ConstantInteger32 *OffsetOp = Mem->getOffset()) {
-      const int32_t Offset = OffsetOp->getValue();
-      if (!isLegalMemOffset(SrcTy, Offset + StackAdjust)) {
-        assert(Mem->getBase()->hasReg());
-        assert(Mem->getBase()->getRegNum() == (int32_t)getFrameOrStackReg());
-        Legalized = true;
-        Src = legalizeStackSlot(SrcTy, Offset, StackAdjust, OrigBaseReg,
-                                NewBaseReg, NewBaseOffset);
-      }
+      _ldr(Dest, createMemOperand(DestTy, Offset, StackAdjust, OrigBaseReg,
+                                  NewBaseReg, NewBaseOffset),
+           MovInstr->getPredicate());
+      Legalized = true;
     }
   }
 
   if (Legalized) {
     if (MovInstr->isDestRedefined()) {
-      _mov_redefined(Dest, Src, MovInstr->getPredicate());
-    } else {
-      _mov(Dest, Src, MovInstr->getPredicate());
+      _set_dest_redefined();
     }
     MovInstr->setDeleted();
   }
 }
 
 void TargetARM32::legalizeStackSlots() {
   // If a stack variable's frame offset doesn't fit, convert from:
   //   ldr X, OFF[SP]
   // to:
   //   movw/movt TMP, OFF_PART
   //   add TMP, TMP, SP
   //   ldr X, OFF_MORE[TMP]
   //
   // This is safe because we have reserved TMP, and add for ARM does not
   // clobber the flags register.
   Func->dump("Before legalizeStackSlots");
   assert(hasComputedFrame());
-  // Early exit, if SpillAreaSizeBytes is really small.
-  // TODO(jpp): this is not safe -- loads and stores of q registers can't have
-  // offsets.
-  if (isLegalMemOffset(IceType_v4i32, SpillAreaSizeBytes))
-    return;
   Variable *OrigBaseReg = getPhysicalRegister(getFrameOrStackReg());
   int32_t StackAdjust = 0;
   // Do a fairly naive greedy clustering for now. Pick the first stack slot
   // that's out of bounds and make a new base reg using the architecture's temp
   // register. If that works for the next slot, then great. Otherwise, create a
   // new base register, clobbering the previous base register. Never share a
   // base reg across different basic blocks. This isn't ideal if local and
   // multi-block variables are far apart and their references are interspersed.
   // It may help to be more coordinated about assign stack slot numbers and may
   // help to assign smaller offsets to higher-weight variables so that they
@@ skipping 22 matching lines @@
           NewBaseOffset += AdjInst->getAmount();
           continue;
         }
         if (llvm::isa<InstARM32Call>(CurInstr)) {
           NewBaseOffset -= StackAdjust;
           StackAdjust = 0;
           continue;
         }
       }
 
-      // The Lowering ensures that ldr and str always have legal Mem operands.
-      // The only other instruction that may access memory is mov.
       if (auto *MovInstr = llvm::dyn_cast<InstARM32Mov>(CurInstr)) {
-        legalizeMovStackAddrImm(MovInstr, StackAdjust, OrigBaseReg, &NewBaseReg,
-                                &NewBaseOffset);
+        legalizeMov(MovInstr, StackAdjust, OrigBaseReg, &NewBaseReg,
+                    &NewBaseOffset);
       }
     }
   }
 }
 
 Operand *TargetARM32::loOperand(Operand *Operand) {
   assert(Operand->getType() == IceType_i64);
   if (Operand->getType() != IceType_i64)
     return Operand;
   if (auto *Var64On32 = llvm::dyn_cast<Variable64On32>(Operand))
@@ skipping 3453 matching lines @@
   Context.insert(InstFakeDef::create(Func, Reg));
   UnimplementedError(Func->getContext()->getFlags());
   return Reg;
 }
 
 // Helper for legalize() to emit the right code to lower an operand to a
 // register of the appropriate type.
 Variable *TargetARM32::copyToReg(Operand *Src, int32_t RegNum) {
   Type Ty = Src->getType();
   Variable *Reg = makeReg(Ty, RegNum);
-  _mov(Reg, Src);
+  if (auto *Mem = llvm::dyn_cast<OperandARM32Mem>(Src)) {
+    _ldr(Reg, Mem);
+  } else {
+    _mov(Reg, Src);
+  }
   return Reg;
 }
 
 Operand *TargetARM32::legalize(Operand *From, LegalMask Allowed,
                                int32_t RegNum) {
   Type Ty = From->getType();
   // Assert that a physical register is allowed. To date, all calls to
   // legalize() allow a physical register. Legal_Flex converts registers to the
   // right type OperandARM32FlexReg as needed.
   assert(Allowed & Legal_Reg);
@@ skipping 32 matching lines @@
     if (Index) {
       assert(Offset == nullptr);
       assert(MemTraits[Ty].CanHaveIndex);
       RegIndex = legalizeToReg(Index);
     }
     if (Offset && Offset->getValue() != 0) {
       assert(Index == nullptr);
       static constexpr bool ZeroExt = false;
       assert(MemTraits[Ty].CanHaveImm);
       if (!OperandARM32Mem::canHoldOffset(Ty, ZeroExt, Offset->getValue())) {
-        assert(RegBase->hasReg());
-        assert(RegBase->getRegNum() == (int32_t)getFrameOrStackReg());
+        assert(false);

[Karl, 2015/11/18 18:40:42]

Why have code below this? Doesn't assert always fail (except for release
compile, which assumes this won't happen)?

[John, 2015/11/19 13:41:22]

It used to be the case that TargetARM32::legalizeMov() would take care of these.
This assert was just a "heads up" to me, during development.

These were introduced by TargetARM32::finishArgumentLowering(). I hae changed
the implementation so that these are now emitted as _mov(Var, Mem()), which is
later converted to _ldr(Var, Mem()) during the mov legalization.

         // We are a bit more lenient with invalid immediate when accessing the
         // stack here, and then rely on legalizeStackSlots() to fix things as
         // appropriate.
         InvalidImm = true;
       }
     }
 
     // Create a new operand if there was a change.
     if (Base != RegBase || Index != RegIndex) {
       // There is only a reg +/- reg or reg + imm form.
@@ skipping 744 matching lines @@
   // Technically R9 is used for TLS with Sandboxing, and we reserve it.
   // However, for compatibility with current NaCl LLVM, don't claim that.
   Str << ".eabi_attribute 14, 3   @ Tag_ABI_PCS_R9_use: Not used\n";
 }
 
 llvm::SmallBitVector TargetARM32::TypeToRegisterSet[IceType_NUM];
 llvm::SmallBitVector TargetARM32::RegisterAliases[RegARM32::Reg_NUM];
 llvm::SmallBitVector TargetARM32::ScratchRegs;
 
 } // end of namespace Ice