[PATCH] D34311: [InstCombine] Don't replace allocas with globals we didn't prove that the global is large enough.

Vitaly Buka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 16 17:58:50 PDT 2017


vitalybuka created this revision.

Assuming that MemTransferInst is correct, the length arg of the instruction is
smaller than available part of the global. So we can have this trivially just
compare it with alloca size.

This simplified check can prevent some correct optimizations. However I see that
new check prevents optimization for 6 of 1147 allocas on llvm code (I compile
and test llvm, clang and compiler-rt). So we more sophisticated check should not
be necessary.

If we need more precise check we need to know that replaced alloca is not larger
than the global replacing it. Seems it's not trivial if source of the
MemTransferInst is pointing into a middle of the global.

Fixes PR33372


https://reviews.llvm.org/D34311

Files:
  lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
  test/Transforms/InstCombine/memcpy-from-global.ll


Index: test/Transforms/InstCombine/memcpy-from-global.ll
===================================================================
--- test/Transforms/InstCombine/memcpy-from-global.ll
+++ test/Transforms/InstCombine/memcpy-from-global.ll
@@ -204,3 +204,32 @@
 ; CHECK-NEXT: call void @bar(i8* bitcast (%U* getelementptr inbounds ([2 x %U], [2 x %U]* @H, i64 0, i64 1) to i8*))
   ret void
 }
+
+ at bbb = local_unnamed_addr global [1000000 x i8] zeroinitializer, align 16
+ at _ZL3KKK = internal unnamed_addr constant [3 x i8] c"\01\01\02", align 1
+
+define void @test9_small_global() {
+; CHECK-LABEL: @test9_small_global(
+; CHECK-NOT: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}@bbb,{{.*}}@_ZL3KKK, 
+; CHECK: alloca [1000000 x i8]
+entry:
+  %cc = alloca [1000000 x i8], align 16
+  %cc.0..sroa_idx = getelementptr inbounds [1000000 x i8], [1000000 x i8]* %cc, i64 0, i64 0
+  %arraydecay = getelementptr inbounds [1000000 x i8], [1000000 x i8]* %cc, i32 0, i32 0
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %arraydecay, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZL3KKK, i32 0, i32 0), i64 3, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds ([1000000 x i8], [1000000 x i8]* @bbb, i32 0, i32 0), i8* %arraydecay, i64 1000000, i32 16, i1 false)
+  ret void
+}
+
+define void @test10_same_global() {
+; CHECK-LABEL: @test10_same_global(
+; CHECK-NOT: alloca
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}@bbb,{{.*}}@_ZL3KKK,{{.*}}, i64 3, i32 1, i1 false)
+entry:
+  %cc = alloca [3 x i8], align 1
+  %cc.0..sroa_idx = getelementptr inbounds [3 x i8], [3 x i8]* %cc, i64 0, i64 0
+  %arraydecay = getelementptr inbounds [3 x i8], [3 x i8]* %cc, i32 0, i32 0
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %arraydecay, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZL3KKK, i32 0, i32 0), i64 3, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds ([1000000 x i8], [1000000 x i8]* @bbb, i32 0, i32 0), i8* %arraydecay, i64 3, i32 1, i1 false)
+  ret void
+}
Index: lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
===================================================================
--- lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
+++ lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
@@ -30,7 +30,6 @@
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
-#include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Type.h"
Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -157,14 +157,45 @@
   return true;
 }
 
+/// Returns true if MemTransferInst overwrites entire alloca.
+static bool
+isCompletlyOverwrittenByConstantGlobal(const AllocaInst &AI,
+                                       const MemTransferInst &TheCopy) {
+  const ConstantInt *ArraySize = dyn_cast<ConstantInt>(AI.getArraySize());
+  if (!ArraySize)
+    return false;
+
+  uint64_t ConstArraySize = ArraySize->getZExtValue();
+  if (!ConstArraySize)
+    return false;
+
+  const DataLayout &DL = AI.getModule()->getDataLayout();
+  uint64_t TySize = DL.getTypeStoreSize(AI.getAllocatedType());
+  if (!TySize)
+    return false;
+
+  ConstantInt *CopyLength = dyn_cast<ConstantInt>(TheCopy.getLength());
+  if (!CopyLength)
+    return false;
+
+  uint64_t CopySize = CopyLength->getZExtValue();
+  uint64_t AllocaSize = TySize * ConstArraySize;
+  assert(CopySize <= AllocaSize);
+  if (CopySize < AllocaSize)
+    return false;
+
+  return true;
+}
+
 /// isOnlyCopiedFromConstantGlobal - Return true if the specified alloca is only
 /// modified by a copy from a constant global.  If we can prove this, we can
 /// replace any uses of the alloca with uses of the global directly.
 static MemTransferInst *
 isOnlyCopiedFromConstantGlobal(AllocaInst *AI,
                                SmallVectorImpl<Instruction *> &ToDelete) {
   MemTransferInst *TheCopy = nullptr;
-  if (isOnlyCopiedFromConstantGlobal(AI, TheCopy, ToDelete))
+  if (isOnlyCopiedFromConstantGlobal(AI, TheCopy, ToDelete) && TheCopy &&
+      isCompletlyOverwrittenByConstantGlobal(*AI, *TheCopy))
     return TheCopy;
   return nullptr;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34311.102918.patch
Type: text/x-patch
Size: 4389 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170617/9f039c3b/attachment.bin>


More information about the llvm-commits mailing list