Fix PR2423 by checking all indices for out of range access, not only
[oota-llvm.git] / lib / Transforms / Scalar / ScalarReplAggregates.cpp
index c80f050730a04073d6974643527058b0e9c46bc3..c5ca22145e3f7cd99826ab2651b045eb1867bb65 100644 (file)
@@ -118,13 +118,17 @@ namespace {
     const Type *CanConvertToScalar(Value *V, bool &IsNotTrivial);
     void ConvertToScalar(AllocationInst *AI, const Type *Ty);
     void ConvertUsesToScalar(Value *Ptr, AllocaInst *NewAI, unsigned Offset);
+    Value *ConvertUsesOfLoadToScalar(LoadInst *LI, AllocaInst *NewAI, 
+                                     unsigned Offset);
+    Value *ConvertUsesOfStoreToScalar(StoreInst *SI, AllocaInst *NewAI, 
+                                      unsigned Offset);
     static Instruction *isOnlyCopiedFromConstantGlobal(AllocationInst *AI);
   };
-
-  char SROA::ID = 0;
-  RegisterPass<SROA> X("scalarrepl", "Scalar Replacement of Aggregates");
 }
 
+char SROA::ID = 0;
+static RegisterPass<SROA> X("scalarrepl", "Scalar Replacement of Aggregates");
+
 // Public interface to the ScalarReplAggregates pass
 FunctionPass *llvm::createScalarReplAggregatesPass(signed int Threshold) { 
   return new SROA(Threshold);
@@ -174,6 +178,14 @@ bool SROA::performPromotion(Function &F) {
   return Changed;
 }
 
+/// getNumSAElements - Return the number of elements in the specific struct or
+/// array.
+static uint64_t getNumSAElements(const Type *T) {
+  if (const StructType *ST = dyn_cast<StructType>(T))
+    return ST->getNumElements();
+  return cast<ArrayType>(T)->getNumElements();
+}
+
 // performScalarRepl - This algorithm is a simple worklist driven algorithm,
 // which runs on all of the malloc/alloca instructions in the function, removing
 // them if they are only used by getelementptr instructions.
@@ -220,7 +232,10 @@ bool SROA::performScalarRepl(Function &F) {
         (isa<StructType>(AI->getAllocatedType()) ||
          isa<ArrayType>(AI->getAllocatedType())) &&
         AI->getAllocatedType()->isSized() &&
-        TD.getABITypeSize(AI->getAllocatedType()) < SRThreshold) {
+        // Do not promote any struct whose size is larger than "128" bytes.
+        TD.getABITypeSize(AI->getAllocatedType()) < SRThreshold &&
+        // Do not promote any struct into more than "32" separate vars.
+        getNumSAElements(AI->getAllocatedType()) < SRThreshold/4) {
       // Check that all of the users of the allocation are capable of being
       // transformed.
       switch (isSafeAllocaToScalarRepl(AI)) {
@@ -298,6 +313,43 @@ void SROA::DoScalarReplacement(AllocationInst *AI,
       continue;
     }
     
+    // Replace:
+    //   %res = load { i32, i32 }* %alloc
+    // with:
+    //   %load.0 = load i32* %alloc.0
+    //   %insert.0 insertvalue { i32, i32 } zeroinitializer, i32 %load.0, 0 
+    //   %load.1 = load i32* %alloc.1
+    //   %insert = insertvalue { i32, i32 } %insert.0, i32 %load.1, 1 
+    // (Also works for arrays instead of structs)
+    if (LoadInst *LI = dyn_cast<LoadInst>(User)) {
+      Value *Insert = UndefValue::get(LI->getType());
+      for (unsigned i = 0, e = ElementAllocas.size(); i != e; ++i) {
+        Value *Load = new LoadInst(ElementAllocas[i], "load", LI);
+        Insert = InsertValueInst::Create(Insert, Load, i, "insert", LI);
+      }
+      LI->replaceAllUsesWith(Insert);
+      LI->eraseFromParent();
+      continue;
+    }
+
+    // Replace:
+    //   store { i32, i32 } %val, { i32, i32 }* %alloc
+    // with:
+    //   %val.0 = extractvalue { i32, i32 } %val, 0 
+    //   store i32 %val.0, i32* %alloc.0
+    //   %val.1 = extractvalue { i32, i32 } %val, 1 
+    //   store i32 %val.1, i32* %alloc.1
+    // (Also works for arrays instead of structs)
+    if (StoreInst *SI = dyn_cast<StoreInst>(User)) {
+      Value *Val = SI->getOperand(0);
+      for (unsigned i = 0, e = ElementAllocas.size(); i != e; ++i) {
+        Value *Extract = ExtractValueInst::Create(Val, i, Val->getName(), SI);
+        new StoreInst(Extract, ElementAllocas[i], SI);
+      }
+      SI->eraseFromParent();
+      continue;
+    }
+    
     GetElementPtrInst *GEPI = cast<GetElementPtrInst>(User);
     // We now know that the GEP is of the form: GEP <ptr>, 0, <cst>
     unsigned Idx =
@@ -319,8 +371,8 @@ void SROA::DoScalarReplacement(AllocationInst *AI,
       SmallVector<Value*, 8> NewArgs;
       NewArgs.push_back(Constant::getNullValue(Type::Int32Ty));
       NewArgs.append(GEPI->op_begin()+3, GEPI->op_end());
-      RepValue = new GetElementPtrInst(AllocaToUse, NewArgs.begin(),
-                                       NewArgs.end(), "", GEPI);
+      RepValue = GetElementPtrInst::Create(AllocaToUse, NewArgs.begin(),
+                                           NewArgs.end(), "", GEPI);
       RepValue->takeName(GEPI);
     }
     
@@ -436,6 +488,12 @@ void SROA::isSafeUseOfAllocation(Instruction *User, AllocationInst *AI,
   if (BitCastInst *C = dyn_cast<BitCastInst>(User))
     return isSafeUseOfBitCastedAllocation(C, AI, Info);
 
+  if (isa<LoadInst>(User))
+    return; // Loads (returning a first class aggregrate) are always rewritable
+
+  if (isa<StoreInst>(User) && User->getOperand(0) != AI)
+    return; // Store is ok if storing INTO the pointer, not storing the pointer
   GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(User);
   if (GEPI == 0)
     return MarkUnsafe(Info);
@@ -453,42 +511,12 @@ void SROA::isSafeUseOfAllocation(Instruction *User, AllocationInst *AI,
 
   bool IsAllZeroIndices = true;
   
-  // If this is a use of an array allocation, do a bit more checking for sanity.
+  // If the first index is a non-constant index into an array, see if we can
+  // handle it as a special case.
   if (const ArrayType *AT = dyn_cast<ArrayType>(*I)) {
-    uint64_t NumElements = AT->getNumElements();
-
-    if (ConstantInt *Idx = dyn_cast<ConstantInt>(I.getOperand())) {
-      IsAllZeroIndices &= Idx->isZero();
-      
-      // Check to make sure that index falls within the array.  If not,
-      // something funny is going on, so we won't do the optimization.
-      //
-      if (Idx->getZExtValue() >= NumElements)
-        return MarkUnsafe(Info);
-
-      // We cannot scalar repl this level of the array unless any array
-      // sub-indices are in-range constants.  In particular, consider:
-      // A[0][i].  We cannot know that the user isn't doing invalid things like
-      // allowing i to index an out-of-range subscript that accesses A[1].
-      //
-      // Scalar replacing *just* the outer index of the array is probably not
-      // going to be a win anyway, so just give up.
-      for (++I; I != E && (isa<ArrayType>(*I) || isa<VectorType>(*I)); ++I) {
-        uint64_t NumElements;
-        if (const ArrayType *SubArrayTy = dyn_cast<ArrayType>(*I))
-          NumElements = SubArrayTy->getNumElements();
-        else
-          NumElements = cast<VectorType>(*I)->getNumElements();
-        
-        ConstantInt *IdxVal = dyn_cast<ConstantInt>(I.getOperand());
-        if (!IdxVal) return MarkUnsafe(Info);
-        if (IdxVal->getZExtValue() >= NumElements)
-          return MarkUnsafe(Info);
-        IsAllZeroIndices &= IdxVal->isZero();
-      }
-      
-    } else {
+    if (!isa<ConstantInt>(I.getOperand())) {
       IsAllZeroIndices = 0;
+      uint64_t NumElements = AT->getNumElements();
       
       // If this is an array index and the index is not constant, we cannot
       // promote... that is unless the array has exactly one or two elements in
@@ -502,7 +530,33 @@ void SROA::isSafeUseOfAllocation(Instruction *User, AllocationInst *AI,
       return MarkUnsafe(Info);
     }
   }
-
+  
+  
+  // Walk through the GEP type indices, checking the types that this indexes
+  // into.
+  for (; I != E; ++I) {
+    // Ignore struct elements, no extra checking needed for these.
+    if (isa<StructType>(*I))
+      continue;
+    
+    // Don't SROA pointers into vectors.
+    if (isa<VectorType>(*I))
+      return MarkUnsafe(Info);
+    
+    // Otherwise, we must have an index into an array type.  Verify that this is
+    // an in-range constant integer.  Specifically, consider A[0][i].  We
+    // cannot know that the user isn't doing invalid things like allowing i to
+    // index an out-of-range subscript that accesses A[1].  Because of this, we
+    // have to reject SROA of any accesses into structs where any of the
+    // components are variables.
+    ConstantInt *IdxVal = dyn_cast<ConstantInt>(I.getOperand());
+    if (!IdxVal) return MarkUnsafe(Info);
+    if (IdxVal->getZExtValue() >= cast<ArrayType>(*I)->getNumElements())
+      return MarkUnsafe(Info);
+    
+    IsAllZeroIndices &= IdxVal->isZero();
+  }
+  
   // If there are any non-simple uses of this getelementptr, make sure to reject
   // them.
   return isSafeElementUse(GEPI, IsAllZeroIndices, AI, Info);
@@ -627,19 +681,17 @@ void SROA::RewriteBitCastUserOfAlloca(Instruction *BCInst, AllocationInst *AI,
       // If this is a memcpy/memmove, emit a GEP of the other element address.
       Value *OtherElt = 0;
       if (OtherPtr) {
-        Value *Idx[2];
-        Idx[0] = Zero;
-        Idx[1] = ConstantInt::get(Type::Int32Ty, i);
-        OtherElt = new GetElementPtrInst(OtherPtr, Idx, Idx + 2,
-                                         OtherPtr->getNameStr()+"."+utostr(i),
-                                         MI);
+        Value *Idx[2] = { Zero, ConstantInt::get(Type::Int32Ty, i) };
+        OtherElt = GetElementPtrInst::Create(OtherPtr, Idx, Idx + 2,
+                                           OtherPtr->getNameStr()+"."+utostr(i),
+                                             MI);
       }
 
       Value *EltPtr = NewElts[i];
       const Type *EltTy =cast<PointerType>(EltPtr->getType())->getElementType();
       
       // If we got down to a scalar, insert a load or store as appropriate.
-      if (EltTy->isFirstClassType()) {
+      if (EltTy->isSingleValueType()) {
         if (isa<MemCpyInst>(MI) || isa<MemMoveInst>(MI)) {
           Value *Elt = new LoadInst(SROADest ? OtherElt : EltPtr, "tmp",
                                     MI);
@@ -712,7 +764,7 @@ void SROA::RewriteBitCastUserOfAlloca(Instruction *BCInst, AllocationInst *AI,
           ConstantInt::get(MI->getOperand(3)->getType(), EltSize), // Size
           Zero  // Align
         };
-        new CallInst(TheFn, Ops, Ops + 4, "", MI);
+        CallInst::Create(TheFn, Ops, Ops + 4, "", MI);
       } else {
         assert(isa<MemSetInst>(MI));
         Value *Ops[] = {
@@ -720,7 +772,7 @@ void SROA::RewriteBitCastUserOfAlloca(Instruction *BCInst, AllocationInst *AI,
           ConstantInt::get(MI->getOperand(3)->getType(), EltSize), // Size
           Zero  // Align
         };
-        new CallInst(TheFn, Ops, Ops + 4, "", MI);
+        CallInst::Create(TheFn, Ops, Ops + 4, "", MI);
       }
     }
 
@@ -733,8 +785,7 @@ void SROA::RewriteBitCastUserOfAlloca(Instruction *BCInst, AllocationInst *AI,
 
 /// HasPadding - Return true if the specified type has any structure or
 /// alignment padding, false otherwise.
-static bool HasPadding(const Type *Ty, const TargetData &TD,
-                       bool inPacked = false) {
+static bool HasPadding(const Type *Ty, const TargetData &TD) {
   if (const StructType *STy = dyn_cast<StructType>(Ty)) {
     const StructLayout *SL = TD.getStructLayout(STy);
     unsigned PrevFieldBitOffset = 0;
@@ -742,7 +793,7 @@ static bool HasPadding(const Type *Ty, const TargetData &TD,
       unsigned FieldBitOffset = SL->getElementOffsetInBits(i);
 
       // Padding in sub-elements?
-      if (HasPadding(STy->getElementType(i), TD, STy->isPacked()))
+      if (HasPadding(STy->getElementType(i), TD))
         return true;
 
       // Check to see if there is any padding between this element and the
@@ -766,12 +817,11 @@ static bool HasPadding(const Type *Ty, const TargetData &TD,
     }
 
   } else if (const ArrayType *ATy = dyn_cast<ArrayType>(Ty)) {
-    return HasPadding(ATy->getElementType(), TD, false);
+    return HasPadding(ATy->getElementType(), TD);
   } else if (const VectorType *VTy = dyn_cast<VectorType>(Ty)) {
-    return HasPadding(VTy->getElementType(), TD, false);
+    return HasPadding(VTy->getElementType(), TD);
   }
-  return inPacked ?
-    false : TD.getTypeSizeInBits(Ty) != TD.getABITypeSizeInBits(Ty);
+  return TD.getTypeSizeInBits(Ty) != TD.getABITypeSizeInBits(Ty);
 }
 
 /// isSafeStructAllocaToScalarRepl - Check to see if the specified allocation of
@@ -834,22 +884,22 @@ void SROA::CanonicalizeAllocaUsers(AllocationInst *AI) {
           // Insert the new GEP instructions, which are properly indexed.
           SmallVector<Value*, 8> Indices(GEPI->op_begin()+1, GEPI->op_end());
           Indices[1] = Constant::getNullValue(Type::Int32Ty);
-          Value *ZeroIdx = new GetElementPtrInst(GEPI->getOperand(0),
-                                                 Indices.begin(),
-                                                 Indices.end(),
-                                                 GEPI->getName()+".0", GEPI);
+          Value *ZeroIdx = GetElementPtrInst::Create(GEPI->getOperand(0),
+                                                     Indices.begin(),
+                                                     Indices.end(),
+                                                     GEPI->getName()+".0", GEPI);
           Indices[1] = ConstantInt::get(Type::Int32Ty, 1);
-          Value *OneIdx = new GetElementPtrInst(GEPI->getOperand(0),
-                                                Indices.begin(),
-                                                Indices.end(),
-                                                GEPI->getName()+".1", GEPI);
+          Value *OneIdx = GetElementPtrInst::Create(GEPI->getOperand(0),
+                                                    Indices.begin(),
+                                                    Indices.end(),
+                                                    GEPI->getName()+".1", GEPI);
           // Replace all loads of the variable index GEP with loads from both
           // indexes and a select.
           while (!GEPI->use_empty()) {
             LoadInst *LI = cast<LoadInst>(GEPI->use_back());
             Value *Zero = new LoadInst(ZeroIdx, LI->getName()+".0", LI);
             Value *One  = new LoadInst(OneIdx , LI->getName()+".1", LI);
-            Value *R = new SelectInst(IsOne, One, Zero, LI->getName(), LI);
+            Value *R = SelectInst::Create(IsOne, One, Zero, LI->getName(), LI);
             LI->replaceAllUsesWith(R);
             LI->eraseFromParent();
           }
@@ -959,12 +1009,22 @@ const Type *SROA::CanConvertToScalar(Value *V, bool &IsNotTrivial) {
     Instruction *User = cast<Instruction>(*UI);
     
     if (LoadInst *LI = dyn_cast<LoadInst>(User)) {
+      // FIXME: Loads of a first class aggregrate value could be converted to a
+      // series of loads and insertvalues
+      if (!LI->getType()->isSingleValueType())
+        return 0;
+
       if (MergeInType(LI->getType(), UsedType, TD))
         return 0;
       
     } else if (StoreInst *SI = dyn_cast<StoreInst>(User)) {
       // Storing the pointer, not into the value?
       if (SI->getOperand(0) == V) return 0;
+
+      // FIXME: Stores of a first class aggregrate value could be converted to a
+      // series of extractvalues and stores
+      if (!SI->getOperand(0)->getType()->isSingleValueType())
+        return 0;
       
       // NOTE: We could handle storing of FP imms into integers here!
       
@@ -1071,163 +1131,17 @@ void SROA::ConvertToScalar(AllocationInst *AI, const Type *ActualTy) {
 /// Offset is an offset from the original alloca, in bits that need to be
 /// shifted to the right.  By the end of this, there should be no uses of Ptr.
 void SROA::ConvertUsesToScalar(Value *Ptr, AllocaInst *NewAI, unsigned Offset) {
-  const TargetData &TD = getAnalysis<TargetData>();
   while (!Ptr->use_empty()) {
     Instruction *User = cast<Instruction>(Ptr->use_back());
     
     if (LoadInst *LI = dyn_cast<LoadInst>(User)) {
-      // The load is a bit extract from NewAI shifted right by Offset bits.
-      Value *NV = new LoadInst(NewAI, LI->getName(), LI);
-      if (NV->getType() == LI->getType() && Offset == 0) {
-        // We win, no conversion needed.
-      } else if (const VectorType *PTy = dyn_cast<VectorType>(NV->getType())) {
-        // If the result alloca is a vector type, this is either an element
-        // access or a bitcast to another vector type.
-        if (isa<VectorType>(LI->getType())) {
-          NV = new BitCastInst(NV, LI->getType(), LI->getName(), LI);
-        } else {
-          // Must be an element access.
-          unsigned Elt = Offset/TD.getABITypeSizeInBits(PTy->getElementType());
-          NV = new ExtractElementInst(
-                         NV, ConstantInt::get(Type::Int32Ty, Elt), "tmp", LI);
-        }
-      } else if (isa<PointerType>(NV->getType())) {
-        assert(isa<PointerType>(LI->getType()));
-        // Must be ptr->ptr cast.  Anything else would result in NV being
-        // an integer.
-        NV = new BitCastInst(NV, LI->getType(), LI->getName(), LI);
-      } else {
-        const IntegerType *NTy = cast<IntegerType>(NV->getType());
-
-        // If this is a big-endian system and the load is narrower than the
-        // full alloca type, we need to do a shift to get the right bits.
-        int ShAmt = 0;
-        if (TD.isBigEndian()) {
-          // On big-endian machines, the lowest bit is stored at the bit offset
-          // from the pointer given by getTypeStoreSizeInBits.  This matters for
-          // integers with a bitwidth that is not a multiple of 8.
-          ShAmt = TD.getTypeStoreSizeInBits(NTy) -
-            TD.getTypeStoreSizeInBits(LI->getType()) - Offset;
-        } else {
-          ShAmt = Offset;
-        }
-
-        // Note: we support negative bitwidths (with shl) which are not defined.
-        // We do this to support (f.e.) loads off the end of a structure where
-        // only some bits are used.
-        if (ShAmt > 0 && (unsigned)ShAmt < NTy->getBitWidth())
-          NV = BinaryOperator::createLShr(NV, 
-                                          ConstantInt::get(NV->getType(),ShAmt),
-                                          LI->getName(), LI);
-        else if (ShAmt < 0 && (unsigned)-ShAmt < NTy->getBitWidth())
-          NV = BinaryOperator::createShl(NV, 
-                                         ConstantInt::get(NV->getType(),-ShAmt),
-                                         LI->getName(), LI);
-        
-        // Finally, unconditionally truncate the integer to the right width.
-        unsigned LIBitWidth = TD.getTypeSizeInBits(LI->getType());
-        if (LIBitWidth < NTy->getBitWidth())
-          NV = new TruncInst(NV, IntegerType::get(LIBitWidth),
-                             LI->getName(), LI);
-        
-        // If the result is an integer, this is a trunc or bitcast.
-        if (isa<IntegerType>(LI->getType())) {
-          assert(NV->getType() == LI->getType() && "Truncate wasn't enough?");
-        } else if (LI->getType()->isFloatingPoint()) {
-          // Just do a bitcast, we know the sizes match up.
-          NV = new BitCastInst(NV, LI->getType(), LI->getName(), LI);
-        } else {
-          // Otherwise must be a pointer.
-          NV = new IntToPtrInst(NV, LI->getType(), LI->getName(), LI);
-        }
-      }
+      Value *NV = ConvertUsesOfLoadToScalar(LI, NewAI, Offset);
       LI->replaceAllUsesWith(NV);
       LI->eraseFromParent();
     } else if (StoreInst *SI = dyn_cast<StoreInst>(User)) {
       assert(SI->getOperand(0) != Ptr && "Consistency error!");
 
-      // Convert the stored type to the actual type, shift it left to insert
-      // then 'or' into place.
-      Value *SV = SI->getOperand(0);
-      const Type *AllocaType = NewAI->getType()->getElementType();
-      if (SV->getType() == AllocaType) {
-        // All is well.
-      } else if (const VectorType *PTy = dyn_cast<VectorType>(AllocaType)) {
-        Value *Old = new LoadInst(NewAI, NewAI->getName()+".in", SI);
-
-        // If the result alloca is a vector type, this is either an element
-        // access or a bitcast to another vector type.
-        if (isa<VectorType>(SV->getType())) {
-          SV = new BitCastInst(SV, AllocaType, SV->getName(), SI);
-        } else {
-          // Must be an element insertion.
-          unsigned Elt = Offset/TD.getABITypeSizeInBits(PTy->getElementType());
-          SV = new InsertElementInst(Old, SV,
-                                     ConstantInt::get(Type::Int32Ty, Elt),
-                                     "tmp", SI);
-        }
-      } else if (isa<PointerType>(AllocaType)) {
-        // If the alloca type is a pointer, then all the elements must be
-        // pointers.
-        if (SV->getType() != AllocaType)
-          SV = new BitCastInst(SV, AllocaType, SV->getName(), SI);
-      } else {
-        Value *Old = new LoadInst(NewAI, NewAI->getName()+".in", SI);
-
-        // If SV is a float, convert it to the appropriate integer type.
-        // If it is a pointer, do the same, and also handle ptr->ptr casts
-        // here.
-        unsigned SrcWidth = TD.getTypeSizeInBits(SV->getType());
-        unsigned DestWidth = TD.getTypeSizeInBits(AllocaType);
-        unsigned SrcStoreWidth = TD.getTypeStoreSizeInBits(SV->getType());
-        unsigned DestStoreWidth = TD.getTypeStoreSizeInBits(AllocaType);
-        if (SV->getType()->isFloatingPoint())
-          SV = new BitCastInst(SV, IntegerType::get(SrcWidth),
-                               SV->getName(), SI);
-        else if (isa<PointerType>(SV->getType()))
-          SV = new PtrToIntInst(SV, TD.getIntPtrType(), SV->getName(), SI);
-                 
-        // Always zero extend the value if needed.
-        if (SV->getType() != AllocaType)
-          SV = new ZExtInst(SV, AllocaType, SV->getName(), SI);
-
-        // If this is a big-endian system and the store is narrower than the
-        // full alloca type, we need to do a shift to get the right bits.
-        int ShAmt = 0;
-        if (TD.isBigEndian()) {
-          // On big-endian machines, the lowest bit is stored at the bit offset
-          // from the pointer given by getTypeStoreSizeInBits.  This matters for
-          // integers with a bitwidth that is not a multiple of 8.
-          ShAmt = DestStoreWidth - SrcStoreWidth - Offset;
-        } else {
-          ShAmt = Offset;
-        }
-
-        // Note: we support negative bitwidths (with shr) which are not defined.
-        // We do this to support (f.e.) stores off the end of a structure where
-        // only some bits in the structure are set.
-        APInt Mask(APInt::getLowBitsSet(DestWidth, SrcWidth));
-        if (ShAmt > 0 && (unsigned)ShAmt < DestWidth) {
-          SV = BinaryOperator::createShl(SV, 
-                                         ConstantInt::get(SV->getType(), ShAmt),
-                                         SV->getName(), SI);
-          Mask <<= ShAmt;
-        } else if (ShAmt < 0 && (unsigned)-ShAmt < DestWidth) {
-          SV = BinaryOperator::createLShr(SV,
-                                         ConstantInt::get(SV->getType(),-ShAmt),
-                                          SV->getName(), SI);
-          Mask = Mask.lshr(ShAmt);
-        }
-        
-        // Mask out the bits we are about to insert from the old value, and or
-        // in the new bits.
-        if (SrcWidth != DestWidth) {
-          assert(DestWidth > SrcWidth);
-          Old = BinaryOperator::createAnd(Old, ConstantInt::get(~Mask),
-                                          Old->getName()+".mask", SI);
-          SV = BinaryOperator::createOr(Old, SV, SV->getName()+".ins", SI);
-        }
-      }
+      Value *SV = ConvertUsesOfStoreToScalar(SI, NewAI, Offset);
       new StoreInst(SV, NewAI, SI);
       SI->eraseFromParent();
       
@@ -1279,6 +1193,201 @@ void SROA::ConvertUsesToScalar(Value *Ptr, AllocaInst *NewAI, unsigned Offset) {
   }
 }
 
+/// ConvertUsesOfLoadToScalar - Convert all of the users the specified load to
+/// use the new alloca directly, returning the value that should replace the
+/// load.  This happens when we are converting an "integer union" to a
+/// single integer scalar, or when we are converting a "vector union" to a
+/// vector with insert/extractelement instructions.
+///
+/// Offset is an offset from the original alloca, in bits that need to be
+/// shifted to the right.  By the end of this, there should be no uses of Ptr.
+Value *SROA::ConvertUsesOfLoadToScalar(LoadInst *LI, AllocaInst *NewAI, 
+                                       unsigned Offset) {
+  // The load is a bit extract from NewAI shifted right by Offset bits.
+  Value *NV = new LoadInst(NewAI, LI->getName(), LI);
+  
+  if (NV->getType() == LI->getType() && Offset == 0) {
+    // We win, no conversion needed.
+    return NV;
+  } 
+
+  // If the result type of the 'union' is a pointer, then this must be ptr->ptr
+  // cast.  Anything else would result in NV being an integer.
+  if (isa<PointerType>(NV->getType())) {
+    assert(isa<PointerType>(LI->getType()));
+    return new BitCastInst(NV, LI->getType(), LI->getName(), LI);
+  }
+  
+  if (const VectorType *VTy = dyn_cast<VectorType>(NV->getType())) {
+    // If the result alloca is a vector type, this is either an element
+    // access or a bitcast to another vector type.
+    if (isa<VectorType>(LI->getType()))
+      return new BitCastInst(NV, LI->getType(), LI->getName(), LI);
+
+    // Otherwise it must be an element access.
+    const TargetData &TD = getAnalysis<TargetData>();
+    unsigned Elt = 0;
+    if (Offset) {
+      unsigned EltSize = TD.getABITypeSizeInBits(VTy->getElementType());
+      Elt = Offset/EltSize;
+      Offset -= EltSize*Elt;
+    }
+    NV = new ExtractElementInst(NV, ConstantInt::get(Type::Int32Ty, Elt),
+                                "tmp", LI);
+    
+    // If we're done, return this element.
+    if (NV->getType() == LI->getType() && Offset == 0)
+      return NV;
+  }
+  
+  const IntegerType *NTy = cast<IntegerType>(NV->getType());
+  
+  // If this is a big-endian system and the load is narrower than the
+  // full alloca type, we need to do a shift to get the right bits.
+  int ShAmt = 0;
+  const TargetData &TD = getAnalysis<TargetData>();
+  if (TD.isBigEndian()) {
+    // On big-endian machines, the lowest bit is stored at the bit offset
+    // from the pointer given by getTypeStoreSizeInBits.  This matters for
+    // integers with a bitwidth that is not a multiple of 8.
+    ShAmt = TD.getTypeStoreSizeInBits(NTy) -
+    TD.getTypeStoreSizeInBits(LI->getType()) - Offset;
+  } else {
+    ShAmt = Offset;
+  }
+  
+  // Note: we support negative bitwidths (with shl) which are not defined.
+  // We do this to support (f.e.) loads off the end of a structure where
+  // only some bits are used.
+  if (ShAmt > 0 && (unsigned)ShAmt < NTy->getBitWidth())
+    NV = BinaryOperator::CreateLShr(NV, 
+                                    ConstantInt::get(NV->getType(),ShAmt),
+                                    LI->getName(), LI);
+  else if (ShAmt < 0 && (unsigned)-ShAmt < NTy->getBitWidth())
+    NV = BinaryOperator::CreateShl(NV, 
+                                   ConstantInt::get(NV->getType(),-ShAmt),
+                                   LI->getName(), LI);
+  
+  // Finally, unconditionally truncate the integer to the right width.
+  unsigned LIBitWidth = TD.getTypeSizeInBits(LI->getType());
+  if (LIBitWidth < NTy->getBitWidth())
+    NV = new TruncInst(NV, IntegerType::get(LIBitWidth),
+                       LI->getName(), LI);
+  
+  // If the result is an integer, this is a trunc or bitcast.
+  if (isa<IntegerType>(LI->getType())) {
+    // Should be done.
+  } else if (LI->getType()->isFloatingPoint()) {
+    // Just do a bitcast, we know the sizes match up.
+    NV = new BitCastInst(NV, LI->getType(), LI->getName(), LI);
+  } else {
+    // Otherwise must be a pointer.
+    NV = new IntToPtrInst(NV, LI->getType(), LI->getName(), LI);
+  }
+  assert(NV->getType() == LI->getType() && "Didn't convert right?");
+  return NV;
+}
+
+
+/// ConvertUsesOfStoreToScalar - Convert the specified store to a load+store
+/// pair of the new alloca directly, returning the value that should be stored
+/// to the alloca.  This happens when we are converting an "integer union" to a
+/// single integer scalar, or when we are converting a "vector union" to a
+/// vector with insert/extractelement instructions.
+///
+/// Offset is an offset from the original alloca, in bits that need to be
+/// shifted to the right.  By the end of this, there should be no uses of Ptr.
+Value *SROA::ConvertUsesOfStoreToScalar(StoreInst *SI, AllocaInst *NewAI, 
+                                        unsigned Offset) {
+  
+  // Convert the stored type to the actual type, shift it left to insert
+  // then 'or' into place.
+  Value *SV = SI->getOperand(0);
+  const Type *AllocaType = NewAI->getType()->getElementType();
+  if (SV->getType() == AllocaType && Offset == 0) {
+    // All is well.
+  } else if (const VectorType *PTy = dyn_cast<VectorType>(AllocaType)) {
+    Value *Old = new LoadInst(NewAI, NewAI->getName()+".in", SI);
+    
+    // If the result alloca is a vector type, this is either an element
+    // access or a bitcast to another vector type.
+    if (isa<VectorType>(SV->getType())) {
+      SV = new BitCastInst(SV, AllocaType, SV->getName(), SI);
+    } else {
+      // Must be an element insertion.
+      const TargetData &TD = getAnalysis<TargetData>();
+      unsigned Elt = Offset/TD.getABITypeSizeInBits(PTy->getElementType());
+      SV = InsertElementInst::Create(Old, SV,
+                                     ConstantInt::get(Type::Int32Ty, Elt),
+                                     "tmp", SI);
+    }
+  } else if (isa<PointerType>(AllocaType)) {
+    // If the alloca type is a pointer, then all the elements must be
+    // pointers.
+    if (SV->getType() != AllocaType)
+      SV = new BitCastInst(SV, AllocaType, SV->getName(), SI);
+  } else {
+    Value *Old = new LoadInst(NewAI, NewAI->getName()+".in", SI);
+    
+    // If SV is a float, convert it to the appropriate integer type.
+    // If it is a pointer, do the same, and also handle ptr->ptr casts
+    // here.
+    const TargetData &TD = getAnalysis<TargetData>();
+    unsigned SrcWidth = TD.getTypeSizeInBits(SV->getType());
+    unsigned DestWidth = TD.getTypeSizeInBits(AllocaType);
+    unsigned SrcStoreWidth = TD.getTypeStoreSizeInBits(SV->getType());
+    unsigned DestStoreWidth = TD.getTypeStoreSizeInBits(AllocaType);
+    if (SV->getType()->isFloatingPoint())
+      SV = new BitCastInst(SV, IntegerType::get(SrcWidth),
+                           SV->getName(), SI);
+    else if (isa<PointerType>(SV->getType()))
+      SV = new PtrToIntInst(SV, TD.getIntPtrType(), SV->getName(), SI);
+    
+    // Always zero extend the value if needed.
+    if (SV->getType() != AllocaType)
+      SV = new ZExtInst(SV, AllocaType, SV->getName(), SI);
+    
+    // If this is a big-endian system and the store is narrower than the
+    // full alloca type, we need to do a shift to get the right bits.
+    int ShAmt = 0;
+    if (TD.isBigEndian()) {
+      // On big-endian machines, the lowest bit is stored at the bit offset
+      // from the pointer given by getTypeStoreSizeInBits.  This matters for
+      // integers with a bitwidth that is not a multiple of 8.
+      ShAmt = DestStoreWidth - SrcStoreWidth - Offset;
+    } else {
+      ShAmt = Offset;
+    }
+    
+    // Note: we support negative bitwidths (with shr) which are not defined.
+    // We do this to support (f.e.) stores off the end of a structure where
+    // only some bits in the structure are set.
+    APInt Mask(APInt::getLowBitsSet(DestWidth, SrcWidth));
+    if (ShAmt > 0 && (unsigned)ShAmt < DestWidth) {
+      SV = BinaryOperator::CreateShl(SV, 
+                                     ConstantInt::get(SV->getType(), ShAmt),
+                                     SV->getName(), SI);
+      Mask <<= ShAmt;
+    } else if (ShAmt < 0 && (unsigned)-ShAmt < DestWidth) {
+      SV = BinaryOperator::CreateLShr(SV,
+                                      ConstantInt::get(SV->getType(),-ShAmt),
+                                      SV->getName(), SI);
+      Mask = Mask.lshr(ShAmt);
+    }
+    
+    // Mask out the bits we are about to insert from the old value, and or
+    // in the new bits.
+    if (SrcWidth != DestWidth) {
+      assert(DestWidth > SrcWidth);
+      Old = BinaryOperator::CreateAnd(Old, ConstantInt::get(~Mask),
+                                      Old->getName()+".mask", SI);
+      SV = BinaryOperator::CreateOr(Old, SV, SV->getName()+".ins", SI);
+    }
+  }
+  return SV;
+}
+
+
 
 /// PointsToConstantGlobal - Return true if V (possibly indirectly) points to
 /// some part of a constant global variable.  This intentionally only accepts