Subzero: Align the stack at the point of function calls.

src/IceTargetLoweringX8632.cpp

 //===- subzero/src/IceTargetLoweringX8632.cpp - x86-32 lowering -----------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the TargetLoweringX8632 class, which
@@ skipping 106 matching lines @@
   size_t Index = static_cast<size_t>(Ty);
   (void)Index;
   assert(Index < TableTypeX8632AttributesSize);
   return TableTypeX8632Attributes[Ty].InVectorElementType;
 }
 
 // The maximum number of arguments to pass in XMM registers
 const unsigned X86_MAX_XMM_ARGS = 4;
 // The number of bits in a byte
 const unsigned X86_CHAR_BIT = 8;
+// Stack alignment
+const unsigned X86_STACK_ALIGNMENT_BYTES = 16;
+// Size of the return address on the stack
+const unsigned X86_RET_IP_SIZE_BYTES = 4;
+
+// Return the difference between Size and the next highest multiple of
+// the stack alignment. All values are in bytes.
+uint32_t getAdjustmentToAlignStackInBytes(uint32_t Size) {

[Jim Stichnoth, 2014/08/05 18:09:27]

Almost all the calls to this routine have a pattern like:

  foo += getAdjustmentToAlignStackInBytes(foo);

I think it might simplify things if you did it something like this:

  uint32_t applyStackAlignment(uint32_t Value) {
    assert((X86_STACK_ALIGNMENT_BYTES & (X86_STACK_ALIGNMENT_BYTES - 1) == 0);
// power of 2
    return (Value + X86_STACK_ALIGNMENT_BYTES - 1) & ~X86_STACK_ALIGNMENT_BYTES;
  }

  foo = applyStackAlignment(foo);

[wala, 2014/08/05 23:57:03]

Done.

+  uint32_t Offset = Size % X86_STACK_ALIGNMENT_BYTES;
+  if (Offset) {
+    return X86_STACK_ALIGNMENT_BYTES - Offset;
+  }
+  return 0;
+}
 
 // Instruction set options
 namespace cl = ::llvm::cl;
 cl::opt<TargetX8632::X86InstructionSet> CLInstructionSet(
     "mattr", cl::desc("X86 target attributes"),
     cl::init(TargetX8632::SSE2),
     cl::values(
         clEnumValN(TargetX8632::SSE2, "sse2",
                    "Enable SSE2 instructions (default)"),
         clEnumValN(TargetX8632::SSE4_1, "sse4.1",
@@ skipping 104 matching lines @@
   STATIC_ASSERT(_table1_##tag == _table2_##tag);
     ICETYPE_TABLE;
 #undef X
   }
 }
 
 } // end of anonymous namespace
 
 TargetX8632::TargetX8632(Cfg *Func)
     : TargetLowering(Func), InstructionSet(CLInstructionSet),
-      IsEbpBasedFrame(false), FrameSizeLocals(0), LocalsSizeBytes(0),
-      NextLabelNumber(0), ComputedLiveRanges(false),
+      IsEbpBasedFrame(false), NeedsStackAlignment(false), FrameSizeLocals(0),
+      LocalsSizeBytes(0), NextLabelNumber(0), ComputedLiveRanges(false),
       PhysicalRegisters(VarList(Reg_NUM)) {
   // TODO: Don't initialize IntegerRegisters and friends every time.
   // Instead, initialize in some sort of static initializer for the
   // class.
   llvm::SmallBitVector IntegerRegisters(Reg_NUM);
   llvm::SmallBitVector IntegerRegistersI8(Reg_NUM);
   llvm::SmallBitVector FloatRegisters(Reg_NUM);
   llvm::SmallBitVector VectorRegisters(Reg_NUM);
   llvm::SmallBitVector InvalidRegisters(Reg_NUM);
   ScratchRegs.resize(Reg_NUM);
@@ skipping 273 matching lines @@
   Variable *Lo = Arg->getLo();
   Variable *Hi = Arg->getHi();
   Type Ty = Arg->getType();
   if (Lo && Hi && Ty == IceType_i64) {
     assert(Lo->getType() != IceType_i64); // don't want infinite recursion
     assert(Hi->getType() != IceType_i64); // don't want infinite recursion
     finishArgumentLowering(Lo, FramePtr, BasicFrameOffset, InArgsSizeBytes);
     finishArgumentLowering(Hi, FramePtr, BasicFrameOffset, InArgsSizeBytes);
     return;
   }
+  if (isVectorType(Ty)) {
+    InArgsSizeBytes += getAdjustmentToAlignStackInBytes(InArgsSizeBytes);
+  }
   Arg->setStackOffset(BasicFrameOffset + InArgsSizeBytes);
   InArgsSizeBytes += typeWidthInBytesOnStack(Ty);
   if (Arg->hasReg()) {
     assert(Ty != IceType_i64);
     OperandX8632Mem *Mem = OperandX8632Mem::create(
         Func, Ty, FramePtr,
         Ctx->getConstantInt(IceType_i32, Arg->getStackOffset()));
     if (isVectorType(Arg->getType())) {
       _movp(Arg, Mem);
     } else {
       _mov(Arg, Mem);
     }
   }
 }
 
 Type TargetX8632::stackSlotType() { return IceType_i32; }
 
 void TargetX8632::addProlog(CfgNode *Node) {
   // If SimpleCoalescing is false, each variable without a register
   // gets its own unique stack slot, which leads to large stack
   // frames.  If SimpleCoalescing is true, then each "global" variable
   // without a register gets its own slot, but "local" variable slots
   // are reused across basic blocks.  E.g., if A and B are local to
   // block 1 and C is local to block 2, then C may share a slot with A
   // or B.
   const bool SimpleCoalescing = true;
   size_t InArgsSizeBytes = 0;
-  size_t RetIpSizeBytes = 4;
   size_t PreservedRegsSizeBytes = 0;
   LocalsSizeBytes = 0;
   Context.init(Node);
   Context.setInsertPoint(Context.getCur());
 
   // Determine stack frame offsets for each Variable without a
   // register assignment.  This can be done as one variable per stack
   // slot.  Or, do coalescing by running the register allocator again
   // with an infinite set of registers (as a side effect, this gives
   // variables a second chance at physical register assignment).
@@ skipping 66 matching lines @@
     assert((RegsUsed & getRegisterSet(RegSet_FramePointer, RegSet_None))
                .count() == 0);
     PreservedRegsSizeBytes += 4;
     Variable *ebp = getPhysicalRegister(Reg_ebp);
     Variable *esp = getPhysicalRegister(Reg_esp);
     const bool SuppressStackAdjustment = true;
     _push(ebp, SuppressStackAdjustment);
     _mov(ebp, esp);
   }
 
+  if (NeedsStackAlignment) {
+    LocalsSizeBytes += getAdjustmentToAlignStackInBytes(
+        X86_RET_IP_SIZE_BYTES + LocalsSizeBytes + PreservedRegsSizeBytes);
+  }
+
   // Generate "sub esp, LocalsSizeBytes"
   if (LocalsSizeBytes)
     _sub(getPhysicalRegister(Reg_esp),
          Ctx->getConstantInt(IceType_i32, LocalsSizeBytes));
 
   resetStackAdjustment();
 
   // Fill in stack offsets for stack args, and copy args into registers
   // for those that were register-allocated.  Args are pushed right to
   // left, so Arg[0] is closest to the stack/frame pointer.
   Variable *FramePtr = getPhysicalRegister(getFrameOrStackReg());
-  size_t BasicFrameOffset = PreservedRegsSizeBytes + RetIpSizeBytes;
+  size_t BasicFrameOffset = PreservedRegsSizeBytes + X86_RET_IP_SIZE_BYTES;
   if (!IsEbpBasedFrame)
     BasicFrameOffset += LocalsSizeBytes;
 
   unsigned NumXmmArgs = 0;
   for (SizeT i = 0; i < Args.size(); ++i) {
     Variable *Arg = Args[i];
     // Skip arguments passed in registers.
     if (isVectorType(Arg->getType()) && NumXmmArgs < X86_MAX_XMM_ARGS) {
       ++NumXmmArgs;
       continue;
@@ skipping 270 matching lines @@
 
   REGX8632_TABLE
 
 #undef X
 
   return Registers;
 }
 
 void TargetX8632::lowerAlloca(const InstAlloca *Inst) {
   IsEbpBasedFrame = true;
-  // TODO(sehr,stichnot): align allocated memory, keep stack aligned, minimize
-  // the number of adjustments of esp, etc.
+  NeedsStackAlignment = true;

[Jim Stichnoth, 2014/08/05 18:09:27]

A couple things here.

1. It looks like alloca lowering is ignoring InstAlloca::getAlignInBytes().  As
long as you're dealing with alignment here, could you fix that? :)

2. It's not clear to me that the presence of an alloca instruction necessarily
requires 16-byte stack alignment.  Also, if e.g. an alloca with 16*K byte
alignment dominates all calls, then maybe NeedsStackAlignment could be reset to
false for the whole method.  Probably for now you can just add a comment that
we're taking the conservative approach for now.

[wala, 2014/08/05 23:57:03]

Done.

+  // TODO(sehr,stichnot): align allocated memory, minimize the number of
+  // adjustments of esp, etc.
+
   Variable *esp = getPhysicalRegister(Reg_esp);
   Operand *TotalSize = legalize(Inst->getSizeInBytes());
   Variable *Dest = Inst->getDest();
-  _sub(esp, TotalSize);
+  if (ConstantInteger *ConstantTotalSize =
+          llvm::dyn_cast<ConstantInteger>(TotalSize)) {
+    uint32_t Value = ConstantTotalSize->getValue();
+    Value += getAdjustmentToAlignStackInBytes(Value);
+    if (Value) {
+      _sub(esp, Ctx->getConstantInt(IceType_i32, Value));
+    }
+  } else {
+    Variable *T = makeReg(IceType_i32);
+    _mov(T, TotalSize);
+    // Non-constant sizes need to be adjusted to the next highest
+    // multiple of the stack alignment at runtime.
+    _add(T, Ctx->getConstantInt(IceType_i32, 15));

[Jim Stichnoth, 2014/08/05 18:09:26]

Should these constants be in terms of X86_STACK_ALIGNMENT_BYTES?

[wala, 2014/08/05 23:57:03]

Done.

+    _and(T, Ctx->getConstantInt(IceType_i32, -16));
+    _sub(esp, T);
+  }
   _mov(Dest, esp);
 }
 
 void TargetX8632::lowerArithmetic(const InstArithmetic *Inst) {
   Variable *Dest = Inst->getDest();
   Operand *Src0 = legalize(Inst->getSrc(0));
   Operand *Src1 = legalize(Inst->getSrc(1));
   if (Dest->getType() == IceType_i64) {
     Variable *DestLo = llvm::cast<Variable>(loOperand(Dest));
     Variable *DestHi = llvm::cast<Variable>(hiOperand(Dest));
@@ skipping 607 matching lines @@
     _br(Inst->getTargetUnconditional());
   } else {
     Operand *Src0 = legalize(Inst->getCondition());
     Constant *Zero = Ctx->getConstantZero(IceType_i32);
     _cmp(Src0, Zero);
     _br(InstX8632Br::Br_ne, Inst->getTargetTrue(), Inst->getTargetFalse());
   }
 }
 
 void TargetX8632::lowerCall(const InstCall *Instr) {
+  // x86-32 calling convention:
+  //
+  // * At the point before the call, the stack must be aligned to 16
+  // bytes.
+  //
+  // * The first four arguments of vector type, regardless of their
+  // position relative to the other arguments in the argument list, are
+  // placed in registers xmm0 - xmm3.
+  //
+  // * Other arguments are placed on the stack ordered according to the

[Jim Stichnoth, 2014/08/05 18:09:27]

This is unclear to me (and possibly wrong).  Does something like this seem
accurate and more precise?

Other arguments are pushed onto the stack in right-to-left order, such that the
left-most argument ends up on the top of the stack at the lowest memory address.

[wala, 2014/08/05 23:57:03]

Done.

+  // argument list and get assigned stack locations with increasing
+  // addresses. The first stack argument is placed at what is the bottom
+  // of the stack at the point before the call.
+  //
+  // * Stack arguments of vector type are aligned to the next highest
+  // multiple of 16 bytes.  Other stack arguments are aligned to the
+  // next highest multiple of 4 bytes.
+  NeedsStackAlignment = true;
+
+  OperandList XmmArgs;
+  OperandList StackArgs, StackArgLocations;
+  uint32_t ParameterAreaSizeBytes = 0;
+
   // Classify each argument operand according to the location where the
   // argument is passed.
-  OperandList XmmArgs;
-  OperandList StackArgs;
   for (SizeT i = 0, NumArgs = Instr->getNumArgs(); i < NumArgs; ++i) {
     Operand *Arg = Instr->getArg(i);
-    if (isVectorType(Arg->getType()) && XmmArgs.size() < X86_MAX_XMM_ARGS) {
+    Type Ty = Arg->getType();
+    // The PNaCl ABI requires the width of arguments to be at least 32 bits.
+    assert(Ty == IceType_i32 || Ty == IceType_f32 || Ty == IceType_i64 ||
+           Ty == IceType_f64 || isVectorType(Ty));
+    if (isVectorType(Ty) && XmmArgs.size() < X86_MAX_XMM_ARGS) {
       XmmArgs.push_back(Arg);
     } else {
       StackArgs.push_back(Arg);
+      if (isVectorType(Arg->getType())) {
+        ParameterAreaSizeBytes +=
+            getAdjustmentToAlignStackInBytes(ParameterAreaSizeBytes);
+      }
+      Variable *esp = Func->getTarget()->getPhysicalRegister(Reg_esp);
+      Constant *Loc = Ctx->getConstantInt(IceType_i32, ParameterAreaSizeBytes);
+      StackArgLocations.push_back(OperandX8632Mem::create(Func, Ty, esp, Loc));
+      ParameterAreaSizeBytes += typeWidthInBytesOnStack(Arg->getType());
     }
   }
-  // For stack arguments, generate a sequence of push instructions,
-  // pushing right to left, keeping track of stack offsets in case a
-  // push involves a stack operand and we are using an esp-based frame.
-  uint32_t StackOffset = 0;
-  // TODO: Consolidate the stack adjustment for function calls by
-  // reserving enough space for the arguments only once.
+
+  // Adjust the parameter area so that the stack is aligned.  It is
+  // assumed that the stack is already aligned at the start of the
+  // calling sequence.
+  ParameterAreaSizeBytes +=
+      getAdjustmentToAlignStackInBytes(ParameterAreaSizeBytes);
+
+  // Subtract the appropriate amount for the argument area.  This also
+  // takes care of setting the stack adjustment during emission.
   //
   // TODO: If for some reason the call instruction gets dead-code
   // eliminated after lowering, we would need to ensure that the
-  // pre-call push instructions and the post-call esp adjustment get
-  // eliminated as well.
-  for (OperandList::reverse_iterator I = StackArgs.rbegin(),
-           E = StackArgs.rend(); I != E; ++I) {
-    Operand *Arg = legalize(*I);
-    if (Arg->getType() == IceType_i64) {
-      _push(hiOperand(Arg));
-      _push(loOperand(Arg));
-    } else if (Arg->getType() == IceType_f64 || isVectorType(Arg->getType())) {
-      // If the Arg turns out to be a memory operand, more than one push
-      // instruction is required.  This ends up being somewhat clumsy in
-      // the current IR, so we use a workaround.  Force the operand into
-      // a (xmm) register, and then push the register.  An xmm register
-      // push is actually not possible in x86, but the Push instruction
-      // emitter handles this by decrementing the stack pointer and
-      // directly writing the xmm register value.
-      _push(legalize(Arg, Legal_Reg));
-    } else {
-      // Otherwise PNaCl requires parameter types to be at least 32-bits.
-      assert(Arg->getType() == IceType_f32 || Arg->getType() == IceType_i32);
-      _push(Arg);
-    }
-    StackOffset += typeWidthInBytesOnStack(Arg->getType());
+  // pre-call and the post-call esp adjustment get eliminated as well.
+  if (ParameterAreaSizeBytes) {
+    _adjust_stack(ParameterAreaSizeBytes);
   }
+
+  // Copy arguments that are passed on the stack to the appropriate
+  // stack locations.
+  for (SizeT i = 0, e = StackArgs.size(); i < e; ++i) {
+    lowerStore(InstStore::create(Func, StackArgs[i], StackArgLocations[i]));
+    // TODO: Consider calling postLower() here to reduce the register
+    // pressure associated with using too many infinite weight
+    // temporaries when lowering the call sequence in -Om1 mode.
+  }
+
   // Copy arguments to be passed in registers to the appropriate
   // registers.
   // TODO: Investigate the impact of lowering arguments passed in
   // registers after lowering stack arguments as opposed to the other
   // way around.  Lowering register arguments after stack arguments may
   // reduce register pressure.  On the other hand, lowering register
   // arguments first (before stack arguments) may result in more compact
   // code, as the memory operand displacements may end up being smaller
   // before any stack adjustment is done.
   for (SizeT i = 0, NumXmmArgs = XmmArgs.size(); i < NumXmmArgs; ++i) {
@@ skipping 43 matching lines @@
     }
   }
   // TODO(stichnot): LEAHACK: remove Legal_All (and use default) once
   // a proper emitter is used.
   Operand *CallTarget = legalize(Instr->getCallTarget(), Legal_All);
   Inst *NewCall = InstX8632Call::create(Func, ReturnReg, CallTarget);
   Context.insert(NewCall);
   if (ReturnRegHi)
     Context.insert(InstFakeDef::create(Func, ReturnRegHi));
 
-  // Add the appropriate offset to esp.
-  if (StackOffset) {
+  // Add the appropriate offset to esp.  The call instruction takes care
+  // of resetting the stack offset during emission.
+  if (ParameterAreaSizeBytes) {
     Variable *esp = Func->getTarget()->getPhysicalRegister(Reg_esp);
-    _add(esp, Ctx->getConstantInt(IceType_i32, StackOffset));
+    _add(esp, Ctx->getConstantInt(IceType_i32, ParameterAreaSizeBytes));
   }
 
   // Insert a register-kill pseudo instruction.
   VarList KilledRegs;
   for (SizeT i = 0; i < ScratchRegs.size(); ++i) {
     if (ScratchRegs[i])
       KilledRegs.push_back(Func->getTarget()->getPhysicalRegister(i));
   }
   Context.insert(InstFakeKill::create(Func, KilledRegs, NewCall));
 
@@ skipping 458 matching lines @@
   bool CanUsePextr =
       Ty == IceType_v8i16 || Ty == IceType_v8i1 || InstructionSet >= SSE4_1;
   if (CanUsePextr && Ty != IceType_v4f32) {
     // Use pextrb, pextrw, or pextrd.
     Constant *Mask = Ctx->getConstantInt(IceType_i8, Index);
     Variable *SourceVectR = legalizeToVar(SourceVectNotLegalized);
     _pextr(ExtractedElementR, SourceVectR, Mask);
   } else if (Ty == IceType_v4i32 || Ty == IceType_v4f32 || Ty == IceType_v4i1) {
     // Use pshufd and movd/movss.
     //
-    // ALIGNHACK: Force vector operands to registers in instructions that
-    // require aligned memory operands until support for stack alignment
-    // is implemented.
+    // ALIGNHACK: Force vector operands to registers in instructions
+    // that require aligned memory operands until support for data
+    // alignment is implemented.
 #define ALIGN_HACK(Vect) legalizeToVar((Vect))
     Operand *SourceVectRM =
         legalize(SourceVectNotLegalized, Legal_Reg | Legal_Mem);
     Variable *T = NULL;
     if (Index) {
       // The shuffle only needs to occur if the element to be extracted
       // is not at the lowest index.
       Constant *Mask = Ctx->getConstantInt(IceType_i8, Index);
       T = makeReg(Ty);
       _pshufd(T, ALIGN_HACK(SourceVectRM), Mask);
@@ skipping 64 matching lines @@
 
     if (Condition == InstFcmp::True) {
       // makeVectorOfOnes() requires an integer vector type.
       T = makeVectorOfMinusOnes(IceType_v4i32);
     } else if (Condition == InstFcmp::False) {
       T = makeVectorOfZeros(Dest->getType());
     } else {
       Operand *Src0RM = legalize(Src0, Legal_Reg | Legal_Mem);
       Operand *Src1RM = legalize(Src1, Legal_Reg | Legal_Mem);
 
-      // ALIGNHACK: Without support for stack alignment, both operands to
-      // cmpps need to be forced into registers.  Once support for stack
+      // ALIGNHACK: Without support for data alignment, both operands to
+      // cmpps need to be forced into registers.  Once support for data
       // alignment is implemented, remove LEGAL_HACK.
 #define LEGAL_HACK(Vect) legalizeToVar((Vect))
       switch (Condition) {
       default: {
         InstX8632Cmpps::CmppsCond Predicate = TableFcmp[Index].Predicate;
         assert(Predicate != InstX8632Cmpps::Cmpps_Invalid);
         T = makeReg(Src0RM->getType());
         _movp(T, Src0RM);
         _cmpps(T, LEGAL_HACK(Src1RM), Predicate);
       } break;
@@ skipping 119 matching lines @@
       Variable *HighOrderBits = makeVectorOfHighOrderBits(Ty);
       _movp(T0, Src0RM);
       _pxor(T0, HighOrderBits);
       _movp(T1, Src1RM);
       _pxor(T1, HighOrderBits);
       Src0RM = T0;
       Src1RM = T1;
     }
 
     // TODO: ALIGNHACK: Both operands to compare instructions need to be
-    // in registers until stack alignment support is implemented.  Once
-    // there is support for stack alignment, LEGAL_HACK can be removed.
+    // in registers until data alignment support is implemented.  Once
+    // there is support for data alignment, LEGAL_HACK can be removed.
 #define LEGAL_HACK(Vect) legalizeToVar((Vect))
     Variable *T = makeReg(Ty);
     switch (Condition) {
     default:
       llvm_unreachable("unexpected condition");
       break;
     case InstIcmp::Eq: {
       _movp(T, Src0RM);
       _pcmpeq(T, LEGAL_HACK(Src1RM));
     } break;
@@ skipping 199 matching lines @@
     // insertelement into index 3 (result is stored in T):
     //   T := SourceVectRM
     //   ElementR := ElementR[0, 0] T[0, 2]
     //   T := T[0, 1] ElementR[3, 0]
     const unsigned char Mask1[3] = {0, 192, 128};
     const unsigned char Mask2[3] = {227, 196, 52};
 
     Constant *Mask1Constant = Ctx->getConstantInt(IceType_i8, Mask1[Index - 1]);
     Constant *Mask2Constant = Ctx->getConstantInt(IceType_i8, Mask2[Index - 1]);
 
-    // ALIGNHACK: Force vector operands to registers in instructions that
-    // require aligned memory operands until support for stack alignment
-    // is implemented.
+    // ALIGNHACK: Force vector operands to registers in instructions
+    // that require aligned memory operands until support for data
+    // alignment is implemented.
 #define ALIGN_HACK(Vect) legalizeToVar((Vect))
     if (Index == 1) {
       SourceVectRM = ALIGN_HACK(SourceVectRM);
       _shufps(ElementR, SourceVectRM, Mask1Constant);
       _shufps(ElementR, SourceVectRM, Mask2Constant);
       _movp(Inst->getDest(), ElementR);
     } else {
       Variable *T = makeReg(Ty);
       _movp(T, SourceVectRM);
       _shufps(ElementR, T, Mask1Constant);
@@ skipping 267 matching lines @@
   case Intrinsics::Memmove: {
     InstCall *Call = makeHelperCall("memmove", NULL, 3);
     Call->addArg(Instr->getArg(0));
     Call->addArg(Instr->getArg(1));
     Call->addArg(Instr->getArg(2));
     lowerCall(Call);
     return;
   }
   case Intrinsics::Memset: {
     // The value operand needs to be extended to a stack slot size
-    // because "push" only works for a specific operand size.
+    // because the PNaCl ABI requires arguments to be at least 32 bits
+    // wide.
     Operand *ValOp = Instr->getArg(1);
     assert(ValOp->getType() == IceType_i8);
     Variable *ValExt = Func->makeVariable(stackSlotType(), Context.getNode());
     lowerCast(InstCast::create(Func, InstCast::Zext, ValExt, ValOp));
     InstCall *Call = makeHelperCall("memset", NULL, 3);
     Call->addArg(Instr->getArg(0));
     Call->addArg(ValExt);
     Call->addArg(Instr->getArg(2));
     lowerCall(Call);
     return;
@@ skipping 648 matching lines @@
   Variable *Dest = Inst->getDest();
   Operand *SrcT = Inst->getTrueOperand();
   Operand *SrcF = Inst->getFalseOperand();
   Operand *Condition = Inst->getCondition();
 
   if (isVectorType(Dest->getType())) {
     Type SrcTy = SrcT->getType();
     Variable *T = makeReg(SrcTy);
     Operand *SrcTRM = legalize(SrcT, Legal_Reg | Legal_Mem);
     Operand *SrcFRM = legalize(SrcF, Legal_Reg | Legal_Mem);
-    // ALIGNHACK: Until stack alignment support is implemented, vector
+    // ALIGNHACK: Until data alignment support is implemented, vector
     // instructions need to have vector operands in registers.  Once
-    // there is support for stack alignment, LEGAL_HACK can be removed.
+    // there is support for data alignment, LEGAL_HACK can be removed.
 #define LEGAL_HACK(Vect) legalizeToVar((Vect))
     if (InstructionSet >= SSE4_1) {
       // TODO(wala): If the condition operand is a constant, use blendps
       // or pblendw.
       //
       // Use blendvps or pblendvb to implement select.
       if (SrcTy == IceType_v4i1 || SrcTy == IceType_v4i32 ||
           SrcTy == IceType_v4f32) {
         Operand *ConditionRM = legalize(Condition, Legal_Reg | Legal_Mem);
         Variable *xmm0 = makeReg(IceType_v4i32, Reg_xmm0);
@@ skipping 74 matching lines @@
     _mov(Dest, SrcF);
   }
 
   Context.insert(Label);
 }
 
 void TargetX8632::lowerStore(const InstStore *Inst) {
   Operand *Value = Inst->getData();
   Operand *Addr = Inst->getAddr();
   OperandX8632Mem *NewAddr = FormMemoryOperand(Addr, Value->getType());
+  Type Ty = NewAddr->getType();
 
-  if (NewAddr->getType() == IceType_i64) {
+  if (Ty == IceType_i64) {
     Value = legalize(Value);
     Operand *ValueHi = legalize(hiOperand(Value), Legal_Reg | Legal_Imm, true);
     Operand *ValueLo = legalize(loOperand(Value), Legal_Reg | Legal_Imm, true);
     _store(ValueHi, llvm::cast<OperandX8632Mem>(hiOperand(NewAddr)));
     _store(ValueLo, llvm::cast<OperandX8632Mem>(loOperand(NewAddr)));
+  } else if (isVectorType(Ty)) {
+    _storep(legalizeToVar(Value), NewAddr);
   } else {
     Value = legalize(Value, Legal_Reg | Legal_Imm, true);
     _store(Value, NewAddr);
   }
 }
 
 void TargetX8632::doAddressOptStore() {
   InstStore *Inst = llvm::cast<InstStore>(*Context.getCur());
   Operand *Data = Inst->getData();
   Operand *Addr = Inst->getAddr();
@@ skipping 322 matching lines @@
       SizeT NumVars = Src->getNumVars();
       for (SizeT J = 0; J < NumVars; ++J) {
         Variable *Var = Src->getVar(J);
         if (Var->hasReg())
           continue;
         if (!Var->getWeight().isInf())
           continue;
         llvm::SmallBitVector AvailableTypedRegisters =
             AvailableRegisters & getRegisterSetForType(Var->getType());
         if (!AvailableTypedRegisters.any()) {
-          // This is a hack in case we run out of physical registers
-          // due to an excessive number of "push" instructions from
-          // lowering a call.
+          // This is a hack in case we run out of physical registers due
+          // to an excessively long code sequence, as might happen when
+          // lowering arguments in lowerCall().
           AvailableRegisters = WhiteList;
           AvailableTypedRegisters =
               AvailableRegisters & getRegisterSetForType(Var->getType());
         }
         assert(AvailableTypedRegisters.any());
         int32_t RegNum = AvailableTypedRegisters.find_first();
         Var->setRegNum(RegNum);
         AvailableRegisters[RegNum] = false;
       }
     }
@@ skipping 111 matching lines @@
     for (SizeT i = 0; i < Size; ++i) {
       Str << "\t.byte\t" << (((unsigned)Data[i]) & 0xff) << "\n";
     }
     Str << "\t.size\t" << MangledName << ", " << Size << "\n";
   }
   Str << "\t" << (IsInternal ? ".local" : ".global") << "\t" << MangledName
       << "\n";
 }
 
 } // end of namespace Ice