[GVN] permit forwarding to non-byte-sized loads

[vtjnash]

Summary

This test was originally added in, but it's marked xfail in NewGVN. I think this seems to fix it.

commit 7470fb01d00e559eb9bed5fe00ba7feeb427d5f9
Author: Jakub Staszak <[e-mail]>
Date:   Fri Sep 2 14:57:37 2011 +0000

    Compare type size instead of type _store_ size to make sure that BitCastInst
    will be valid. This fixes PR10820.
    
    llvm-svn: 139005

[vtjnash]

fix test

[vtjnash]

use helper method

[vtjnash]

The work had already pretty much been done before (for GVN), we just needed to
allow it to use that work here for NewGVN, and use bits directly rather than
bytes/8*8 for the output computation.

llvm/lib/Transforms/Utils/VNCoercion.cpp

bool canCoerceMustAliasedValueToLoad(Value *StoredVal, Type *LoadTy,
   // don't try to transform them. We need to be able to bitcast to integer.
   if (isFirstClassAggregateOrScalableType(LoadTy) ||
       isFirstClassAggregateOrScalableType(StoredTy))
     return false;
 
   uint64_t StoreSize = DL.getTypeSizeInBits(StoredTy).getFixedSize();
 
   // The store size must be byte-aligned to support future type casts.
-  if (llvm::alignTo(StoreSize, 8) != StoreSize)
+  if (StoreSize & 7)
     return false;
 
   // The store has to be at least as big as the load.
   if (StoreSize < DL.getTypeSizeInBits(LoadTy).getFixedSize())
     return false;
 
   bool StoredNI = DL.isNonIntegralPointerType(StoredTy->getScalarType());
   bool LoadNI = DL.isNonIntegralPointerType(LoadTy->getScalarType());
static T *coerceAvailableValueToLoadTypeHelper(T *StoredVal, Type *LoadedTy,
   }
 
   // Convert vectors and fp to integer, which can be manipulated.
   if (!StoredValTy->isIntegerTy()) {
     StoredValTy = IntegerType::get(StoredValTy->getContext(), StoredValSize);
     StoredVal = Helper.CreateBitCast(StoredVal, StoredValTy);
   }
 
-  // If this is a big-endian system, we need to shift the value down to the low
+  // If this is a big-endian system, we need to shift the bytes down to the low
   // bits so that a truncate will work.
   if (DL.isBigEndian()) {
-    uint64_t ShiftAmt = DL.getTypeStoreSizeInBits(StoredValTy).getFixedSize() -
-                        DL.getTypeStoreSizeInBits(LoadedTy).getFixedSize();
+    uint64_t ShiftBytes = DL.getTypeStoreSize(StoredValTy).getFixedSize() -
+                          DL.getTypeStoreSize(LoadedTy).getFixedSize();
     StoredVal = Helper.CreateLShr(
-        StoredVal, ConstantInt::get(StoredVal->getType(), ShiftAmt));
+        StoredVal, ConstantInt::get(StoredVal->getType(), ShiftBytes * 8));
   }
 
   // Truncate the integer to the right size now.
   Type *NewIntTy = IntegerType::get(StoredValTy->getContext(), LoadedValSize);
   StoredVal = Helper.CreateTruncOrBitCast(StoredVal, NewIntTy);
 
   if (LoadedTy != NewIntTy) {
     // If the result is a pointer, inttoptr.
static int analyzeLoadFromClobberingWrite(Type *LoadTy, Value *LoadPtr,
 
   int64_t StoreOffset = 0, LoadOffset = 0;
   Value *StoreBase =
       GetPointerBaseWithConstantOffset(WritePtr, StoreOffset, DL);
   Value *LoadBase = GetPointerBaseWithConstantOffset(LoadPtr, LoadOffset, DL);
   if (StoreBase != LoadBase)
     return -1;
 
-  uint64_t LoadSize = DL.getTypeSizeInBits(LoadTy).getFixedSize();
-
-  if ((WriteSizeInBits & 7) | (LoadSize & 7))
+  if (WriteSizeInBits & 7) // Shifting the input is not handled.
     return -1;
-  uint64_t StoreSize = WriteSizeInBits / 8; // Convert to bytes.
-  LoadSize /= 8;
+  uint64_t StoreSize = (WriteSizeInBits + 7) / 8; // Convert to bytes.
+  uint64_t LoadSize = DL.getTypeSizeInBits(LoadTy).getFixedSize();
+  LoadSize = (LoadSize + 7) / 8;
 
-  // If the Load isn't completely contained within the stored bits, we don't
-  // have all the bits to feed it.  We could do something crazy in the future
-  // (issue a smaller load then merge the bits in) but this seems unlikely to be
-  // valuable.
+  // If the loaded bytes aren't completely contained within the stored bytes,
+  // we don't have all the bits to feed it.  We could do something crazy in the
+  // future (issue a smaller load then merge the bits in) but this seems
+  // unlikely to be valuable.
   if (StoreOffset > LoadOffset ||
       StoreOffset + int64_t(StoreSize) < LoadOffset + int64_t(LoadSize))
     return -1;
 
   // Okay, we can do this transformation.  Return the number of bytes into the
   // store that the load is.
   return LoadOffset - StoreOffset;
 }
static unsigned getLoadLoadClobberFullWidthSize(const Value *MemLocBase,
   int64_t MemLocEnd = MemLocOffs + MemLocSize;
 
   // If no amount of rounding up will let MemLoc fit into LI, then bail out.
   if (LIOffs + LoadAlign < MemLocEnd)
     return 0;
 
   // This is the size of the load to try.  Start with the next larger power of
   // two.
-  unsigned NewLoadByteSize = LI->getType()->getPrimitiveSizeInBits() / 8U;
+  unsigned NewLoadByteSize = (LI->getType()->getPrimitiveSizeInBits() + 7) / 8U;
   NewLoadByteSize = NextPowerOf2(NewLoadByteSize);
 
   while (true) {
     // If this load size is bigger than our known alignment or would not fit
     // into a native integer register, then we fail.
     if (NewLoadByteSize > LoadAlign ||
         !DL.fitsInLegalInteger(NewLoadByteSize * 8))
       return 0;
static T *getStoreValueForLoadHelper(T *SrcVal, unsigned Offset, Type *LoadTy,
   // so we don't need to do any truncation, etc. This avoids introducing
   // ptrtoint instructions for pointers that may be non-integral.
   if (SrcVal->getType()->isPointerTy() && LoadTy->isPointerTy() &&
       cast<PointerType>(SrcVal->getType())->getAddressSpace() ==
           cast<PointerType>(LoadTy)->getAddressSpace()) {
     return SrcVal;
   }
 
-  uint64_t StoreSize =
-      (DL.getTypeSizeInBits(SrcVal->getType()).getFixedSize() + 7) / 8;
-  uint64_t LoadSize = (DL.getTypeSizeInBits(LoadTy).getFixedSize() + 7) / 8;
+  uint64_t StoreSize = DL.getTypeSizeInBits(SrcVal->getType()).getFixedSize();
+  uint64_t LoadSize = DL.getTypeSizeInBits(LoadTy).getFixedSize();
   // Compute which bits of the stored value are being used by the load.  Convert
   // to an integer type to start with.
   if (SrcVal->getType()->isPtrOrPtrVectorTy())
     SrcVal = Helper.CreatePtrToInt(SrcVal, DL.getIntPtrType(SrcVal->getType()));
   if (!SrcVal->getType()->isIntegerTy())
-    SrcVal = Helper.CreateBitCast(SrcVal, IntegerType::get(Ctx, StoreSize * 8));
+    SrcVal = Helper.CreateBitCast(SrcVal, IntegerType::get(Ctx, StoreSize));
 
-  // Shift the bits to the least significant depending on endianness.
-  unsigned ShiftAmt;
+  // Shift the bytes to the least significant bits so that a truncate will
+  // work, depending on the endian.
+  unsigned ShiftBytes;
   if (DL.isLittleEndian())
-    ShiftAmt = Offset * 8;
+    ShiftBytes = Offset;
   else
-    ShiftAmt = (StoreSize - LoadSize - Offset) * 8;
-  if (ShiftAmt)
-    SrcVal = Helper.CreateLShr(SrcVal,
-                               ConstantInt::get(SrcVal->getType(), ShiftAmt));
+    ShiftBytes = (StoreSize - LoadSize) / 8 - Offset;
+  if (ShiftBytes)
+    SrcVal = Helper.CreateLShr(
+        SrcVal, ConstantInt::get(SrcVal->getType(), ShiftBytes * 8));
 
   if (LoadSize != StoreSize)
-    SrcVal = Helper.CreateTruncOrBitCast(SrcVal,
-                                         IntegerType::get(Ctx, LoadSize * 8));
+    SrcVal =
+        Helper.CreateTruncOrBitCast(SrcVal, IntegerType::get(Ctx, LoadSize));
   return SrcVal;
 }
 
 /// This function is called when we have a memdep query of a load that ends up
 /// being a clobbering store.  This means that the store provides bits used by
 /// the load but the pointers don't must-alias.  Check this case to see if
 /// there is anything more we can do before we give up.
 Value *getStoreValueForLoad(Value *SrcVal, unsigned Offset, Type *LoadTy,
Constant *getConstantLoadValueForLoad(Constant *SrcVal, unsigned Offset,
   return getConstantStoreValueForLoad(SrcVal, Offset, LoadTy, DL);
 }
 
 template <class T, class HelperClass>
 T *getMemInstValueForLoadHelper(MemIntrinsic *SrcInst, unsigned Offset,
                                 Type *LoadTy, HelperClass &Helper,
                                 const DataLayout &DL) {
   LLVMContext &Ctx = LoadTy->getContext();
-  uint64_t LoadSize = DL.getTypeSizeInBits(LoadTy).getFixedSize() / 8;
+  uint64_t LoadSize = DL.getTypeStoreSize(LoadTy).getFixedSize();
 
   // We know that this method is only called when the mem transfer fully
   // provides the bits for the load.
   if (MemSetInst *MSI = dyn_cast<MemSetInst>(SrcInst)) {
     // memset(P, 'x', 1234) -> splat('x'), even if x is a variable, and
     // independently of what the offset is.
     T *Val = cast<T>(MSI->getValue());
     if (LoadSize != 1)

llvm/test/Transforms/GVN/pr10820.ll

-; RUN: opt < %s -basic-aa -gvn -S | FileCheck %s
-
-target datalayout =
-"e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-f128:128:128-n8:16:32:64"
-target triple = "x86_64-unknown-linux-gnu"
+; RUN: opt -mtriple="x86_64-unknown-linux-gnu" --data-layout="e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-f128:128:128-n8:16:32:64" < %s -basic-aa -gvn -S | FileCheck %s
+; RUN: opt -mtriple="powerpc64-unknown-linux-gnu" --data-layout="E-m:e-i64:64-n32:64" < %s -basic-aa -gvn -S | FileCheck %s
 
 @g = external global i31
 
 define void @main() nounwind uwtable {
 entry:
 ; CHECK: store i32
   store i32 402662078, i32* bitcast (i31* @g to i32*), align 8
 ; CHECK-NOT: load i31
   %0 = load i31, i31* @g, align 8
 ; CHECK: store i31
   store i31 %0, i31* undef, align 1
   unreachable
 }
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i1)
+
+define i7 @memset_forward(i7 *%p) {
+; CHECK-LABEL: @memset_forward(
+; CHECK-NEXT:    [[CONV:%.*]] = bitcast i7* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* [[CONV]], i8 -120, i64 1, i1 false)
+; CHECK-NEXT:    ret i7 8
+;
+  %conv = bitcast i7* %p to i8*
+  call void @llvm.memset.p0i8.i64(i8* %conv, i8 -120, i64 1, i1 false)
+  %load = load i7, i7* %p
+  ret i7 %load
+}

llvm/test/Transforms/NewGVN/pr10820-xfail.ll

-; XFAIL: *
-; RUN: opt < %s -basic-aa -newgvn -S | FileCheck %s
-; NewGVN fails this due to missing load coercion
-target datalayout =
-"e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-f128:128:128-n8:16:32:64"
-target triple = "x86_64-unknown-linux-gnu"
-
-@g = external global i31
-
-define void @main() nounwind uwtable {
-entry:
-; CHECK: store i32
-  store i32 402662078, i32* bitcast (i31* @g to i32*), align 8
-; CHECK-NOT: load i31
-  %0 = load i31, i31* @g, align 8
-; CHECK: store i31
-  store i31 %0, i31* undef, align 1
-  unreachable
-}

llvm/test/Transforms/NewGVN/pr10820.ll

+; RUN: opt -mtriple="x86_64-unknown-linux-gnu" --data-layout="e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-f128:128:128-n8:16:32:64" < %s -basic-aa -newgvn -S | FileCheck %s
+; RUN: opt -mtriple="powerpc64-unknown-linux-gnu" --data-layout="E-m:e-i64:64-n32:64" < %s -basic-aa -newgvn -S | FileCheck %s
+
+@g = external global i31
+
+define void @main() nounwind uwtable {
+entry:
+; CHECK: store i32
+  store i32 402662078, i32* bitcast (i31* @g to i32*), align 8
+; CHECK-NOT: load i31
+  %0 = load i31, i31* @g, align 8
+; CHECK: store i31
+  store i31 %0, i31* undef, align 1
+  unreachable
+}
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i1)
+
+define i7 @memset_forward(i7 *%p) {
+; CHECK-LABEL: @memset_forward(
+; CHECK-NEXT:    [[CONV:%.*]] = bitcast i7* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* [[CONV]], i8 -120, i64 1, i1 false)
+; CHECK-NEXT:    ret i7 8
+;
+  %conv = bitcast i7* %p to i8*
+  call void @llvm.memset.p0i8.i64(i8* %conv, i8 -120, i64 1, i1 false)
+  %load = load i7, i7* %p
+  ret i7 %load
+}