[canonicalize] Teach InstCombine to canonicalize loads which are only

ever stored to always use a legal integer type if one is available.

Regardless of whether this particular type is good or bad, it ensures we
don't get weird differences in generated code (and resulting
performance) from "equivalent" patterns that happen to end up using
a slightly different type.

After some discussion on llvmdev it seems everyone generally likes this
canonicalization. However, there may be some parts of LLVM that handle
it poorly and need to be fixed. I have at least verified that this
doesn't impede GVN and instcombine's store-to-load forwarding powers in
any obvious cases. Subtle cases are exactly what we need te flush out if
they remain.

Also note that this IR pattern should already be hitting LLVM from Clang
at least because it is exactly the IR which would be produced if you
used memcpy to copy a pointer or floating point between memory instead
of a variable.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226781 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 56212b04eb049ecc9bb8870ec87ac66528cf037c..54b1af8ea79199e223bc4bbb449bc0662b3747dc 100644 (file)
--- a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -417,6 +417,35 @@ static Instruction *combineLoadToOperationType(InstCombiner &IC, LoadInst &LI) {
   if (LI.use_empty())
     return nullptr;
 
   if (LI.use_empty())
     return nullptr;
 

+  Type *Ty = LI.getType();
+
+  // Try to canonicalize loads which are only ever stored to operate over
+  // integers instead of any other type. We only do this when the loaded type
+  // is sized and has a size exactly the same as its store size and the store
+  // size is a legal integer type.
+  const DataLayout *DL = IC.getDataLayout();
+  if (!Ty->isIntegerTy() && Ty->isSized() && DL &&
+      DL->isLegalInteger(DL->getTypeStoreSizeInBits(Ty)) &&
+      DL->getTypeStoreSizeInBits(Ty) == DL->getTypeSizeInBits(Ty)) {
+    if (std::all_of(LI.user_begin(), LI.user_end(), [&LI](User *U) {
+          auto *SI = dyn_cast<StoreInst>(U);
+          return SI && SI->getPointerOperand() != &LI;
+        })) {
+      LoadInst *NewLoad = combineLoadToNewType(
+          IC, LI,
+          Type::getIntNTy(LI.getContext(), DL->getTypeStoreSizeInBits(Ty)));
+      // Replace all the stores with stores of the newly loaded value.
+      for (auto UI = LI.user_begin(), UE = LI.user_end(); UI != UE;) {
+        auto *SI = cast<StoreInst>(*UI++);
+        IC.Builder->SetInsertPoint(SI);
+        combineStoreToNewValue(IC, *SI, NewLoad);
+        IC.EraseInstFromFunction(*SI);
+      }
+      assert(LI.use_empty() && "Failed to remove all users of the load!");
+      // Return the old load so the combiner can delete it safely.
+      return &LI;
+    }
+  }

 
   // Fold away bit casts of the loaded value by loading the desired type.
   if (LI.hasOneUse())
 
   // Fold away bit casts of the loaded value by loading the desired type.
   if (LI.hasOneUse())

diff --git a/test/Transforms/InstCombine/load.ll b/test/Transforms/InstCombine/load.ll
index b4b7558a1a05af8962ea8a14ef105cc5849823d2..700797ab22cf6da9dfebef535e406ace8d785fb7 100644 (file)
--- a/test/Transforms/InstCombine/load.ll
+++ b/test/Transforms/InstCombine/load.ll
@@ -2,7 +2,7 @@
 
 ; This test makes sure that these instructions are properly eliminated.
 
 
 ; This test makes sure that these instructions are properly eliminated.
 

-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target datalayout = "e-m:e-p:64:64:64-i64:64-f80:128-n8:16:32:64-S128"

 
 @X = constant i32 42           ; <i32*> [#uses=2]
 @X2 = constant i32 47          ; <i32*> [#uses=1]
 
 @X = constant i32 42           ; <i32*> [#uses=2]
 @X2 = constant i32 47          ; <i32*> [#uses=1]


@@ -150,3 +150,53 @@ define i8 @test15(i8 %x, i32 %y) {
   %r = load i8* %g.i8
   ret i8 %r
 }
   %r = load i8* %g.i8
   ret i8 %r
 }

+
+define void @test16(i8* %x, i8* %a, i8* %b, i8* %c) {
+; Check that we canonicalize loads which are only stored to use integer types
+; when there is a valid integer type.
+; CHECK-LABEL: @test16(
+; CHECK: %[[L1:.*]] = load i32*
+; CHECK-NOT: load
+; CHECK: store i32 %[[L1]], i32*
+; CHECK: store i32 %[[L1]], i32*
+; CHECK-NOT: store
+; CHECK: %[[L1:.*]] = load i32*
+; CHECK-NOT: load
+; CHECK: store i32 %[[L1]], i32*
+; CHECK: store i32 %[[L1]], i32*
+; CHECK-NOT: store
+; CHECK: ret
+
+entry:
+  %x.cast = bitcast i8* %x to float*
+  %a.cast = bitcast i8* %a to float*
+  %b.cast = bitcast i8* %b to float*
+  %c.cast = bitcast i8* %c to i32*
+
+  %x1 = load float* %x.cast
+  store float %x1, float* %a.cast
+  store float %x1, float* %b.cast
+
+  %x2 = load float* %x.cast
+  store float %x2, float* %b.cast
+  %x2.cast = bitcast float %x2 to i32
+  store i32 %x2.cast, i32* %c.cast
+
+  ret void
+}
+
+define void @test17(i8** %x, i8 %y) {
+; Check that in cases similar to @test16 we don't try to rewrite a load when
+; its only use is a store but it is used as the pointer to that store rather
+; than the value.
+;
+; CHECK-LABEL: @test17(
+; CHECK: %[[L:.*]] = load i8**
+; CHECK: store i8 %y, i8* %[[L]]
+
+entry:
+  %x.load = load i8** %x
+  store i8 %y, i8* %x.load
+
+  ret void
+}

diff --git a/test/Transforms/InstCombine/struct-assign-tbaa.ll b/test/Transforms/InstCombine/struct-assign-tbaa.ll
index 4e4b3f2a1a888a841c8c1378a97c04ab1cb2ed23..e949640d261e299376b397ae45d9b05c8b6be73e 100644 (file)
--- a/test/Transforms/InstCombine/struct-assign-tbaa.ll
+++ b/test/Transforms/InstCombine/struct-assign-tbaa.ll
@@ -10,8 +10,8 @@ declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i32,
 %struct.test1 = type { float }
 
 ; CHECK: @test
 %struct.test1 = type { float }
 
 ; CHECK: @test

-; CHECK: %2 = load float* %0, align 4, !tbaa !0
-; CHECK: store float %2, float* %1, align 4, !tbaa !0
+; CHECK: %[[LOAD:.*]] = load i32* %{{.*}}, align 4, !tbaa !0
+; CHECK: store i32 %[[LOAD:.*]], i32* %{{.*}}, align 4, !tbaa !0

 ; CHECK: ret
 define void @test1(%struct.test1* nocapture %a, %struct.test1* nocapture %b) {
 entry:
 ; CHECK: ret
 define void @test1(%struct.test1* nocapture %a, %struct.test1* nocapture %b) {
 entry: