Subzero: Align spill locations to natural alignment.

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 111 matching lines @@
 
 // The maximum number of arguments to pass in XMM registers
 const uint32_t X86_MAX_XMM_ARGS = 4;
 // The number of bits in a byte
 const uint32_t X86_CHAR_BIT = 8;
 // Stack alignment
 const uint32_t X86_STACK_ALIGNMENT_BYTES = 16;
 // Size of the return address on the stack
 const uint32_t X86_RET_IP_SIZE_BYTES = 4;
 
-// Value is a size in bytes.  Return Value adjusted to the next highest
-// multiple of the stack alignment.
+// Value and Alignment are in bytes.  Return Value adjusted to the next
+// highest multiple of Alignment.
+uint32_t applyAlignment(uint32_t Value, uint32_t Alignment) {
+  // power of 2
+  assert((Alignment & (Alignment - 1)) == 0);
+  return (Value + Alignment - 1) & -Alignment;
+}
+
+// Value is in bytes. Return Value adjusted to the next highest multiple
+// of the stack alignment.
 uint32_t applyStackAlignment(uint32_t Value) {
-  // power of 2
-  assert((X86_STACK_ALIGNMENT_BYTES & (X86_STACK_ALIGNMENT_BYTES - 1)) == 0);
-  return (Value + X86_STACK_ALIGNMENT_BYTES - 1) & -X86_STACK_ALIGNMENT_BYTES;
+  return applyAlignment(Value, X86_STACK_ALIGNMENT_BYTES);
 }
 
 // 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)"),
@@ skipping 365 matching lines @@
     Variable *RegisterArg = Func->makeVariable(Ty, DefNode, Name);
     RegisterArg->setRegNum(RegNum);
     RegisterArg->setIsArg(Func);
     Arg->setIsArg(Func, false);
 
     Args[I] = RegisterArg;
     Context.insert(InstAssign::create(Func, Arg, RegisterArg));
   }
 }
 
+void TargetX8632::sortByAlignment(VarList &Dest, const VarList &Source) const {
+  typedef std::map<uint32_t, VarList> BucketMap;

[Jim Stichnoth, 2014/08/13 23:59:34]

Use a list<> or vector<> instead of map<> ?

[Jim Stichnoth, 2014/08/14 16:48:29]

Never mind this comment, I misread the code.

[wala, 2014/08/14 17:31:24]

There are only 4 possible buckets (corresponding to the type sizes). I ended up
using an array.

[wala, 2014/08/14 17:51:59]

Actually, there are 3 usable buckets (4 bytes, 8 bytes, 16 bytes).

+  BucketMap Buckets;
+
+  for (VarList::const_iterator I = Source.begin(), E = Source.end(); I != E;
+       ++I) {
+    uint32_t NaturalAlignment = typeWidthInBytesOnStack((*I)->getType());
+    Buckets[NaturalAlignment].push_back(*I);
+  }
+
+  for (BucketMap::reverse_iterator I = Buckets.rbegin(), E = Buckets.rend();
+       I != E; ++I) {
+    VarList &List = I->second;
+    Dest.insert(Dest.end(), List.begin(), List.end());
+  }
+}
+
 // 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 TargetX8632::finishArgumentLowering(Variable *Arg, Variable *FramePtr,
@@ skipping 59 matching lines @@
       getRegisterSet(RegSet_CalleeSave, RegSet_None);
 
   size_t GlobalsSize = 0;
   std::vector<size_t> LocalsSize(Func->getNumNodes());
 
   // Prepass.  Compute RegsUsed, PreservedRegsSizeBytes, and
   // LocalsSizeBytes.
   RegsUsed = llvm::SmallBitVector(CalleeSaves.size());
   const VarList &Variables = Func->getVariables();
   const VarList &Args = Func->getArgs();
+  VarList SpilledVariables, SortedSpilledVariables,
+      VariablesLinkedToSpillSplots;
+
+  // If there is a separate locals area, this specifies the alignment
+  // for it.
+  uint32_t LocalsSlotsAlignmentBytes = 0;

[Jim Stichnoth, 2014/08/13 23:59:34]

At this point, the stack frame layout is starting to get complex enough that it
would be nice to have a beautiful ascii art diagram of the frame like in this CL
description, labeled with the local variables that contain the size of each
section.

[wala, 2014/08/14 17:31:24]

Done.

+  // The entire spill locations area gets aligned to largest natural
+  // alignment of the variables that have a spill slot.
+  uint32_t SpillAreaAlignmentBytes = 0;
   for (VarList::const_iterator I = Variables.begin(), E = Variables.end();
        I != E; ++I) {
     Variable *Var = *I;
     if (Var->hasReg()) {
       RegsUsed[Var->getRegNum()] = true;
       continue;
     }
     // An argument either does not need a stack slot (if passed in a
     // register) or already has one (if passed on the stack).
     if (Var->getIsArg())
       continue;
     // An unreferenced variable doesn't need a stack slot.
     if (ComputedLiveRanges && Var->getLiveRange().isEmpty())
       continue;
     // A spill slot linked to a variable with a stack slot should reuse
     // that stack slot.
     if (Var->getWeight() == RegWeight::Zero && Var->getRegisterOverlap()) {
       if (Variable *Linked = Var->getPreferredRegister()) {
-        if (!Linked->hasReg())
+        if (!Linked->hasReg()) {
+          VariablesLinkedToSpillSplots.push_back(Var);
           continue;
+        }
       }
     }
+    SpilledVariables.push_back(Var);
+  }
+
+  sortByAlignment(SortedSpilledVariables, SpilledVariables);
+  for (VarList::const_iterator I = SortedSpilledVariables.begin(),
+                               E = SortedSpilledVariables.end();
+       I != E; ++I) {
+    Variable *Var = *I;
     size_t Increment = typeWidthInBytesOnStack(Var->getType());
+    if (!SpillAreaAlignmentBytes)
+      SpillAreaAlignmentBytes = Increment;
     if (SimpleCoalescing) {
       if (Var->isMultiblockLife()) {
         GlobalsSize += Increment;
       } else {
         SizeT NodeIndex = Var->getLocalUseNode()->getIndex();
         LocalsSize[NodeIndex] += Increment;
         if (LocalsSize[NodeIndex] > LocalsSizeBytes)
           LocalsSizeBytes = LocalsSize[NodeIndex];
+        if (!LocalsSlotsAlignmentBytes)
+          LocalsSlotsAlignmentBytes = Increment;
       }
     } else {
       LocalsSizeBytes += Increment;
     }
   }
   LocalsSizeBytes += GlobalsSize;
 
   // Add push instructions for preserved registers.
   for (SizeT i = 0; i < CalleeSaves.size(); ++i) {
     if (CalleeSaves[i] && RegsUsed[i]) {
       PreservedRegsSizeBytes += 4;
       const bool SuppressStackAdjustment = true;
       _push(getPhysicalRegister(i), SuppressStackAdjustment);
     }
   }
 
   // Generate "push ebp; mov ebp, esp"
   if (IsEbpBasedFrame) {
     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);
   }
 
+  // Align the variables area.
+  uint32_t SpillAreaPaddingBytes = 0;
+  if (SpillAreaAlignmentBytes) {
+    assert(SpillAreaAlignmentBytes <= X86_STACK_ALIGNMENT_BYTES);
+    uint32_t SpillAreaOffset = X86_RET_IP_SIZE_BYTES + PreservedRegsSizeBytes;
+    uint32_t SpillAreaStart =
+        applyAlignment(SpillAreaOffset, SpillAreaAlignmentBytes);
+    SpillAreaPaddingBytes = SpillAreaStart - SpillAreaOffset;
+    LocalsSizeBytes += SpillAreaPaddingBytes;
+  }
+
+  // If there are separate globals and locals areas, make sure the
+  // locals area is aligned by padding the end of the globals area.
+  if (LocalsSlotsAlignmentBytes) {
+    assert(LocalsSlotsAlignmentBytes <= SpillAreaAlignmentBytes);
+    uint32_t NewGlobalsSize =
+        applyAlignment(GlobalsSize, LocalsSlotsAlignmentBytes);
+    GlobalsSize = NewGlobalsSize;
+    LocalsSizeBytes += NewGlobalsSize - GlobalsSize;

[jvoung (off chromium), 2014/08/14 00:48:49]

Isn't this difference always going to be zero?

Do you want to move the assignment of GlobalsSize = NewGlobalsSize to below this
+= ?

[wala, 2014/08/14 17:31:24]

Ouch, I'm surprised all the tests still passed.

Done: The logic has been changed so that the padding size is properly added to
LocalsSizeBytes. Also, the variables have been renamed for clarity.

I've also added a unit test to exercise this case.

+  }
+
+  // Align esp if necessary.
   if (NeedsStackAlignment) {
-    uint32_t StackSize = applyStackAlignment(
-        X86_RET_IP_SIZE_BYTES + PreservedRegsSizeBytes + LocalsSizeBytes);
-    LocalsSizeBytes =
-        StackSize - X86_RET_IP_SIZE_BYTES - PreservedRegsSizeBytes;
+    uint32_t StackOffset = X86_RET_IP_SIZE_BYTES + PreservedRegsSizeBytes;
+    uint32_t StackSize = applyStackAlignment(StackOffset + LocalsSizeBytes);
+    LocalsSizeBytes = StackSize - StackOffset;
   }
 
   // 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
@@ skipping 13 matching lines @@
       continue;
     }
     finishArgumentLowering(Arg, FramePtr, BasicFrameOffset, InArgsSizeBytes);
   }
 
   // Fill in stack offsets for locals.
   size_t TotalGlobalsSize = GlobalsSize;
   GlobalsSize = 0;
   LocalsSize.assign(LocalsSize.size(), 0);
   size_t NextStackOffset = 0;
-  for (VarList::const_iterator I = Variables.begin(), E = Variables.end();
+  for (VarList::const_iterator I = SortedSpilledVariables.begin(),
+                               E = SortedSpilledVariables.end();
        I != E; ++I) {
     Variable *Var = *I;
-    if (Var->hasReg()) {
-      RegsUsed[Var->getRegNum()] = true;
-      continue;
-    }
-    if (Var->getIsArg())
-      continue;
-    if (ComputedLiveRanges && Var->getLiveRange().isEmpty())
-      continue;
     if (Var->getWeight() == RegWeight::Zero && Var->getRegisterOverlap()) {

[wala, 2014/08/14 17:31:24]

Since I've separated out the list of variables linked to a variable with a spill
slot from the list of spilled variables, I don't think this statement is
necessary, so I've removed it.

       if (Variable *Linked = Var->getPreferredRegister()) {
         if (!Linked->hasReg()) {
           // TODO: Make sure Linked has already been assigned a stack
           // slot.
           Var->setStackOffset(Linked->getStackOffset());
           continue;
         }
       }
     }
     size_t Increment = typeWidthInBytesOnStack(Var->getType());
     if (SimpleCoalescing) {
       if (Var->isMultiblockLife()) {
         GlobalsSize += Increment;
         NextStackOffset = GlobalsSize;
       } else {
         SizeT NodeIndex = Var->getLocalUseNode()->getIndex();
         LocalsSize[NodeIndex] += Increment;
         NextStackOffset = TotalGlobalsSize + LocalsSize[NodeIndex];
       }
     } else {
       NextStackOffset += Increment;
     }
     if (IsEbpBasedFrame)
-      Var->setStackOffset(-NextStackOffset);
-    else
-      Var->setStackOffset(LocalsSizeBytes - NextStackOffset);
+      Var->setStackOffset(-NextStackOffset - SpillAreaPaddingBytes);
+    else {

[Jim Stichnoth, 2014/08/13 23:59:34]

Probably best to have braces around both clauses or neither clause.

[wala, 2014/08/14 17:31:24]

Done.

+      Var->setStackOffset(LocalsSizeBytes - NextStackOffset -
+                          SpillAreaPaddingBytes);
+    }
   }
   this->FrameSizeLocals = NextStackOffset;
   this->HasComputedFrame = true;
 
+  for (VarList::const_iterator I = VariablesLinkedToSpillSplots.begin(),
+                               E = VariablesLinkedToSpillSplots.end();
+       I != E; ++I) {
+    Variable *Var = *I;
+    Variable *Linked = Var->getPreferredRegister();
+    Var->setStackOffset(Linked->getStackOffset());
+  }
+
   if (Func->getContext()->isVerbose(IceV_Frame)) {
     Func->getContext()->getStrDump() << "LocalsSizeBytes=" << LocalsSizeBytes
                                      << "\n"
                                      << "InArgsSizeBytes=" << InArgsSizeBytes
                                      << "\n"
                                      << "PreservedRegsSizeBytes="
                                      << PreservedRegsSizeBytes << "\n";
   }
 }
 
@@ skipping 233 matching lines @@
   assert((AlignmentParam & (AlignmentParam - 1)) == 0);
   assert((X86_STACK_ALIGNMENT_BYTES & (X86_STACK_ALIGNMENT_BYTES - 1)) == 0);
 
   uint32_t Alignment = std::max(AlignmentParam, X86_STACK_ALIGNMENT_BYTES);
   if (Alignment > X86_STACK_ALIGNMENT_BYTES) {
     _and(esp, Ctx->getConstantInt(IceType_i32, -Alignment));
   }
   if (ConstantInteger *ConstantTotalSize =
           llvm::dyn_cast<ConstantInteger>(TotalSize)) {
     uint32_t Value = ConstantTotalSize->getValue();
-    // Round Value up to the next highest multiple of the alignment.
-    Value = (Value + Alignment - 1) & -Alignment;
+    Value = applyAlignment(Value, Alignment);
     _sub(esp, Ctx->getConstantInt(IceType_i32, Value));
   } else {
     // Non-constant sizes need to be adjusted to the next highest
     // multiple of the required alignment at runtime.
     Variable *T = makeReg(IceType_i32);
     _mov(T, TotalSize);
     _add(T, Ctx->getConstantInt(IceType_i32, Alignment - 1));
     _and(T, Ctx->getConstantInt(IceType_i32, -Alignment));
     _sub(esp, T);
   }
@@ skipping 226 matching lines @@
     case InstArithmetic::Fsub:
     case InstArithmetic::Fmul:
     case InstArithmetic::Fdiv:
     case InstArithmetic::Frem:
       llvm_unreachable("FP instruction with i64 type");
       break;
     }
   } else if (isVectorType(Dest->getType())) {
     // TODO: Trap on integer divide and integer modulo by zero.
     // See: https://code.google.com/p/nativeclient/issues/detail?id=3899
-    //
-    // TODO(wala): ALIGNHACK: All vector arithmetic is currently done in
-    // registers.  This is a workaround of the fact that there is no
-    // support for aligning stack operands.  Once there is support,
-    // remove LEGAL_HACK.
-#define LEGAL_HACK(s) legalizeToVar((s))
     switch (Inst->getOp()) {
     case InstArithmetic::_num:
       llvm_unreachable("Unknown arithmetic operator");
       break;
     case InstArithmetic::Add: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _padd(T, LEGAL_HACK(Src1));
+      _padd(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::And: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _pand(T, LEGAL_HACK(Src1));
+      _pand(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Or: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _por(T, LEGAL_HACK(Src1));
+      _por(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Xor: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _pxor(T, LEGAL_HACK(Src1));
+      _pxor(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Sub: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _psub(T, LEGAL_HACK(Src1));
+      _psub(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Mul: {
       bool TypesAreValidForPmull =
           Dest->getType() == IceType_v4i32 || Dest->getType() == IceType_v8i16;
       bool InstructionSetIsValidForPmull =
           Dest->getType() == IceType_v8i16 || InstructionSet >= SSE4_1;
       if (TypesAreValidForPmull && InstructionSetIsValidForPmull) {
         Variable *T = makeReg(Dest->getType());
         _movp(T, Src0);
-        _pmull(T, LEGAL_HACK(Src1));
+        _pmull(T, Src1);
         _movp(Dest, T);
       } else if (Dest->getType() == IceType_v4i32) {
         // Lowering sequence:
         // Note: The mask arguments have index 0 on the left.
         //
         // movups  T1, Src0
         // pshufd  T2, Src0, {1,0,3,0}
         // pshufd  T3, Src1, {1,0,3,0}
         // # T1 = {Src0[0] * Src1[0], Src0[2] * Src1[2]}
         // pmuludq T1, Src1
@@ skipping 12 matching lines @@
         // Dest[0, 2], Src[0, 2]
         const unsigned Mask0202 = 0x88;
         // Mask that directs pshufd to create a vector with entries
         // Src[0, 2, 1, 3]
         const unsigned Mask0213 = 0xd8;
         Variable *T1 = makeReg(IceType_v4i32);
         Variable *T2 = makeReg(IceType_v4i32);
         Variable *T3 = makeReg(IceType_v4i32);
         Variable *T4 = makeReg(IceType_v4i32);
         _movp(T1, Src0);
-        // TODO(wala): ALIGHNHACK: Replace Src0R with Src0 and Src1R
-        // with Src1 after stack operand alignment support is
-        // implemented.
-        Variable *Src0R = LEGAL_HACK(Src0);
-        Variable *Src1R = LEGAL_HACK(Src1);
-        _pshufd(T2, Src0R, Mask1030);
-        _pshufd(T3, Src1R, Mask1030);
-        _pmuludq(T1, Src1R);
+        _pshufd(T2, Src0, Mask1030);
+        _pshufd(T3, Src1, Mask1030);
+        _pmuludq(T1, Src1);
         _pmuludq(T2, T3);
         _shufps(T1, T2, Ctx->getConstantInt(IceType_i8, Mask0202));
         _pshufd(T4, T1, Ctx->getConstantInt(IceType_i8, Mask0213));
         _movp(Dest, T4);
       } else {
         assert(Dest->getType() == IceType_v16i8);
         scalarizeArithmetic(Inst->getOp(), Dest, Src0, Src1);
       }
     } break;
     case InstArithmetic::Shl:
     case InstArithmetic::Lshr:
     case InstArithmetic::Ashr:
     case InstArithmetic::Udiv:
     case InstArithmetic::Urem:
     case InstArithmetic::Sdiv:
     case InstArithmetic::Srem:
       scalarizeArithmetic(Inst->getOp(), Dest, Src0, Src1);
       break;
     case InstArithmetic::Fadd: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _addps(T, LEGAL_HACK(Src1));
+      _addps(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Fsub: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _subps(T, LEGAL_HACK(Src1));
+      _subps(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Fmul: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _mulps(T, LEGAL_HACK(Src1));
+      _mulps(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Fdiv: {
       Variable *T = makeReg(Dest->getType());
       _movp(T, Src0);
-      _divps(T, LEGAL_HACK(Src1));
+      _divps(T, Src1);
       _movp(Dest, T);
     } break;
     case InstArithmetic::Frem:
       scalarizeArithmetic(Inst->getOp(), Dest, Src0, Src1);
       break;
     }
-#undef LEGAL_HACK
   } else { // Dest->getType() is non-i64 scalar
     Variable *T_edx = NULL;
     Variable *T = NULL;
     switch (Inst->getOp()) {
     case InstArithmetic::_num:
       llvm_unreachable("Unknown arithmetic operator");
       break;
     case InstArithmetic::Add:
       _mov(T, Src0);
       _add(T, Src1);
@@ skipping 804 matching lines @@
   // TODO(wala): Determine the best lowering sequences for each type.
   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 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);
+      _pshufd(T, legalize(SourceVectNotLegalized, Legal_Reg | Legal_Mem), Mask);
     } else {
-      T = ALIGN_HACK(SourceVectRM);
+      T = legalizeToVar(SourceVectNotLegalized);
     }
 
     if (InVectorElementTy == IceType_i32) {
       _movd(ExtractedElementR, T);
     } else { // Ty == Icetype_f32
       // TODO(wala): _movss is only used here because _mov does not
       // allow a vector source and a scalar destination.  _mov should be
       // able to be used here.
       // _movss is a binary instruction, so the FakeDef is needed to
       // keep the live range analysis consistent.
       Context.insert(InstFakeDef::create(Func, ExtractedElementR));
       _movss(ExtractedElementR, T);
     }
-#undef ALIGN_HACK
   } else {
     assert(Ty == IceType_v16i8 || Ty == IceType_v16i1);
     // Spill the value to a stack slot and do the extraction in memory.
     //
     // TODO(wala): use legalize(SourceVectNotLegalized, Legal_Mem) when
     // support for legalizing to mem is implemented.
     Variable *Slot = Func->makeVariable(Ty, Context.getNode());
     Slot->setWeight(RegWeight::Zero);
     _movp(Slot, legalizeToVar(SourceVectNotLegalized));
 
@@ skipping 38 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 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);
+        _cmpps(T, Src1RM, Predicate);
       } break;
       case InstFcmp::One: {
         // Check both unequal and ordered.
         T = makeReg(Src0RM->getType());
         Variable *T2 = makeReg(Src0RM->getType());
-        Src1RM = LEGAL_HACK(Src1RM);
         _movp(T, Src0RM);
         _cmpps(T, Src1RM, InstX8632Cmpps::Cmpps_neq);
         _movp(T2, Src0RM);
         _cmpps(T2, Src1RM, InstX8632Cmpps::Cmpps_ord);
         _pand(T, T2);
       } break;
       case InstFcmp::Ueq: {
         // Check both equal or unordered.
         T = makeReg(Src0RM->getType());
         Variable *T2 = makeReg(Src0RM->getType());
-        Src1RM = LEGAL_HACK(Src1RM);
         _movp(T, Src0RM);
         _cmpps(T, Src1RM, InstX8632Cmpps::Cmpps_eq);
         _movp(T2, Src0RM);
         _cmpps(T2, Src1RM, InstX8632Cmpps::Cmpps_unord);
         _por(T, T2);
       } break;
       }
-#undef LEGAL_HACK
     }
 
     _movp(Dest, T);
     eliminateNextVectorSextInstruction(Dest);
     return;
   }
 
   // Lowering a = fcmp cond, b, c
   //   ucomiss b, c       /* only if C1 != Br_None */
   //                      /* but swap b,c order if SwapOperands==true */
@@ skipping 84 matching lines @@
       Variable *T1 = makeReg(Ty);
       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 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));
+      _pcmpeq(T, Src1RM);
     } break;
     case InstIcmp::Ne: {
       _movp(T, Src0RM);
-      _pcmpeq(T, LEGAL_HACK(Src1RM));
+      _pcmpeq(T, Src1RM);
       Variable *MinusOne = makeVectorOfMinusOnes(Ty);
       _pxor(T, MinusOne);
     } break;
     case InstIcmp::Ugt:
     case InstIcmp::Sgt: {
       _movp(T, Src0RM);
-      _pcmpgt(T, LEGAL_HACK(Src1RM));
+      _pcmpgt(T, Src1RM);
     } break;
     case InstIcmp::Uge:
     case InstIcmp::Sge: {
       // !(Src1RM > Src0RM)
       _movp(T, Src1RM);
-      _pcmpgt(T, LEGAL_HACK(Src0RM));
+      _pcmpgt(T, Src0RM);
       Variable *MinusOne = makeVectorOfMinusOnes(Ty);
       _pxor(T, MinusOne);
     } break;
     case InstIcmp::Ult:
     case InstIcmp::Slt: {
       _movp(T, Src1RM);
-      _pcmpgt(T, LEGAL_HACK(Src0RM));
+      _pcmpgt(T, Src0RM);
     } break;
     case InstIcmp::Ule:
     case InstIcmp::Sle: {
       // !(Src0RM > Src1RM)
       _movp(T, Src0RM);
-      _pcmpgt(T, LEGAL_HACK(Src1RM));
+      _pcmpgt(T, Src1RM);
       Variable *MinusOne = makeVectorOfMinusOnes(Ty);
       _pxor(T, MinusOne);
     } break;
     }
-#undef LEGAL_HACK
 
     _movp(Dest, T);
     eliminateNextVectorSextInstruction(Dest);
     return;
   }
 
   // If Src1 is an immediate, or known to be a physical register, we can
   // allow Src0 to be a memory operand.  Otherwise, Src0 must be copied into
   // a physical register.  (Actually, either Src0 or Src1 can be chosen for
   // the physical register, but unfortunately we have to commit to one or
@@ skipping 155 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 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);
       _shufps(T, ElementR, Mask2Constant);
       _movp(Inst->getDest(), T);
     }
-#undef ALIGN_HACK
   } else {
     assert(Ty == IceType_v16i8 || Ty == IceType_v16i1);
     // Spill the value to a stack slot and perform the insertion in
     // memory.
     //
     // TODO(wala): use legalize(SourceVectNotLegalized, Legal_Mem) when
     // support for legalizing to mem is implemented.
     Variable *Slot = Func->makeVariable(Ty, Context.getNode());
     Slot->setWeight(RegWeight::Zero);
     _movp(Slot, legalizeToVar(SourceVectNotLegalized));
@@ skipping 941 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 data alignment support is implemented, vector
-    // instructions need to have vector operands in registers.  Once
-    // 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);
         _movp(xmm0, ConditionRM);
         _psll(xmm0, Ctx->getConstantInt(IceType_i8, 31));
         _movp(T, SrcFRM);
-        _blendvps(T, LEGAL_HACK(SrcTRM), xmm0);
+        _blendvps(T, SrcTRM, xmm0);
         _movp(Dest, T);
       } else {
         assert(typeNumElements(SrcTy) == 8 || typeNumElements(SrcTy) == 16);
         Type SignExtTy = Condition->getType() == IceType_v8i1 ? IceType_v8i16
             : IceType_v16i8;
         Variable *xmm0 = makeReg(SignExtTy, Reg_xmm0);
         lowerCast(InstCast::create(Func, InstCast::Sext, xmm0, Condition));
         _movp(T, SrcFRM);
-        _pblendvb(T, LEGAL_HACK(SrcTRM), xmm0);
+        _pblendvb(T, SrcTRM, xmm0);
         _movp(Dest, T);
       }
       return;
     }
     // Lower select without SSE4.1:
     // a=d?b:c ==>
     //   if elementtype(d) != i1:
     //      d=sext(d);
     //   a=(b&d)|(c&~d);
     Variable *T2 = makeReg(SrcTy);
     // Sign extend the condition operand if applicable.
     if (SrcTy == IceType_v4f32) {
       // The sext operation takes only integer arguments.
       Variable *T3 = Func->makeVariable(IceType_v4i32, Context.getNode());
       lowerCast(InstCast::create(Func, InstCast::Sext, T3, Condition));
       _movp(T, T3);
     } else if (typeElementType(SrcTy) != IceType_i1) {
       lowerCast(InstCast::create(Func, InstCast::Sext, T, Condition));
     } else {
       Operand *ConditionRM = legalize(Condition, Legal_Reg | Legal_Mem);
       _movp(T, ConditionRM);
     }
     _movp(T2, T);
-    _pand(T, LEGAL_HACK(SrcTRM));
-    _pandn(T2, LEGAL_HACK(SrcFRM));
+    _pand(T, SrcTRM);
+    _pandn(T2, SrcFRM);
     _por(T, T2);
     _movp(Dest, T);
-#undef LEGAL_HACK
 
     return;
   }
 
   // a=d?b:c ==> cmp d,0; a=b; jne L1; FakeUse(a); a=c; L1:
   Operand *ConditionRMI = legalize(Condition);
   Constant *Zero = Ctx->getConstantZero(IceType_i32);
   InstX8632Label *Label = InstX8632Label::create(Func, this);
 
   if (Dest->getType() == IceType_i64) {
@@ skipping 542 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