[llvm-commits] [llvm] r61102 - in /llvm/trunk: lib/Transforms/IPO/GlobalOpt.cpp test/Transforms/GlobalOpt/2008-12-16-HeapSRACrash-2.ll
Chris Lattner
sabre at nondot.org
Tue Dec 16 13:24:51 PST 2008
Author: lattner
Date: Tue Dec 16 15:24:51 2008
New Revision: 61102
URL: http://llvm.org/viewvc/llvm-project?rev=61102&view=rev
Log:
Fix another crash found by inspection. If we have a PHI node merging
the load multiple times, make sure the check the uses of the PHI to
ensure they are transformable.
Added:
llvm/trunk/test/Transforms/GlobalOpt/2008-12-16-HeapSRACrash-2.ll
Modified:
llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=61102&r1=61101&r2=61102&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Tue Dec 16 15:24:51 2008
@@ -1012,56 +1012,77 @@
}
}
-/// GlobalLoadUsesSimpleEnoughForHeapSRA - If all users of values loaded from
-/// GV are simple enough to perform HeapSRA, return true.
-static bool GlobalLoadUsesSimpleEnoughForHeapSRA(GlobalVariable *GV,
- MallocInst *MI) {
- for (Value::use_iterator UI = GV->use_begin(), E = GV->use_end(); UI != E;
- ++UI)
- if (LoadInst *LI = dyn_cast<LoadInst>(*UI)) {
- // We permit two users of the load: setcc comparing against the null
- // pointer, and a getelementptr of a specific form.
- for (Value::use_iterator UI = LI->use_begin(), E = LI->use_end();
- UI != E; ++UI) {
- // Comparison against null is ok.
- if (ICmpInst *ICI = dyn_cast<ICmpInst>(*UI)) {
- if (!isa<ConstantPointerNull>(ICI->getOperand(1)))
- return false;
- continue;
- }
-
- // getelementptr is also ok, but only a simple form.
- if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(*UI)) {
- // Must index into the array and into the struct.
- if (GEPI->getNumOperands() < 3)
- return false;
-
- // Otherwise the GEP is ok.
- continue;
- }
+/// LoadUsesSimpleEnoughForHeapSRA - Verify that all uses of V (a load, or a phi
+/// of a load) are simple enough to perform heap SRA on. This permits GEP's
+/// that index through the array and struct field, icmps of null, and PHIs.
+static bool LoadUsesSimpleEnoughForHeapSRA(Value *V, MallocInst *MI,
+ SmallPtrSet<PHINode*, 32> &AnalyzedLoadUsingPHIs) {
+ // We permit two users of the load: setcc comparing against the null
+ // pointer, and a getelementptr of a specific form.
+ for (Value::use_iterator UI = V->use_begin(), E = V->use_end(); UI != E;++UI){
+ Instruction *User = cast<Instruction>(*UI);
+
+ // Comparison against null is ok.
+ if (ICmpInst *ICI = dyn_cast<ICmpInst>(User)) {
+ if (!isa<ConstantPointerNull>(ICI->getOperand(1)))
+ return false;
+ continue;
+ }
+
+ // getelementptr is also ok, but only a simple form.
+ if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(User)) {
+ // Must index into the array and into the struct.
+ if (GEPI->getNumOperands() < 3)
+ return false;
+
+ // Otherwise the GEP is ok.
+ continue;
+ }
+
+ if (PHINode *PN = dyn_cast<PHINode>(User)) {
+ // If we have already recursively analyzed this PHI, then it is safe.
+ if (AnalyzedLoadUsingPHIs.insert(PN))
+ continue;
+
+ // We have a phi of a load from the global. We can only handle this
+ // if the other PHI'd values are actually the same. In this case,
+ // the rewriter will just drop the phi entirely.
+ for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
+ Value *IV = PN->getIncomingValue(i);
+ if (IV == V) continue; // Trivial the same.
- if (PHINode *PN = dyn_cast<PHINode>(*UI)) {
- // We have a phi of a load from the global. We can only handle this
- // if the other PHI'd values are actually the same. In this case,
- // the rewriter will just drop the phi entirely.
- for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
- Value *IV = PN->getIncomingValue(i);
- if (IV == LI) continue; // Trivial the same.
-
- // If the phi'd value is from the malloc that initializes the value,
- // we can xform it.
- if (IV == MI) continue;
-
- // Otherwise, we don't know what it is.
- return false;
- }
- continue;
- }
+ // If the phi'd value is from the malloc that initializes the value,
+ // we can xform it.
+ if (IV == MI) continue;
- // Otherwise we don't know what this is, not ok.
+ // Otherwise, we don't know what it is.
return false;
}
+
+ if (!LoadUsesSimpleEnoughForHeapSRA(PN, MI, AnalyzedLoadUsingPHIs))
+ return false;
+
+ continue;
}
+
+ // Otherwise we don't know what this is, not ok.
+ return false;
+ }
+
+ return true;
+}
+
+
+/// AllGlobalLoadUsesSimpleEnoughForHeapSRA - If all users of values loaded from
+/// GV are simple enough to perform HeapSRA, return true.
+static bool AllGlobalLoadUsesSimpleEnoughForHeapSRA(GlobalVariable *GV,
+ MallocInst *MI) {
+ SmallPtrSet<PHINode*, 32> AnalyzedLoadUsingPHIs;
+ for (Value::use_iterator UI = GV->use_begin(), E = GV->use_end(); UI != E;
+ ++UI)
+ if (LoadInst *LI = dyn_cast<LoadInst>(*UI))
+ if (!LoadUsesSimpleEnoughForHeapSRA(LI, MI, AnalyzedLoadUsingPHIs))
+ return false;
return true;
}
@@ -1166,7 +1187,7 @@
/// RewriteUsesOfLoadForHeapSRoA - We are performing Heap SRoA on a global. Ptr
/// is a value loaded from the global. Eliminate all uses of Ptr, making them
/// use FieldGlobals instead. All uses of loaded values satisfy
-/// GlobalLoadUsesSimpleEnoughForHeapSRA.
+/// AllGlobalLoadUsesSimpleEnoughForHeapSRA.
static void RewriteUsesOfLoadForHeapSRoA(LoadInst *Load,
const std::vector<GlobalVariable*> &FieldGlobals) {
std::vector<Value *> InsertedLoadsForPtr;
@@ -1367,7 +1388,7 @@
// This the structure has an unreasonable number of fields, leave it
// alone.
if (AllocSTy->getNumElements() <= 16 && AllocSTy->getNumElements() != 0 &&
- GlobalLoadUsesSimpleEnoughForHeapSRA(GV, MI)) {
+ AllGlobalLoadUsesSimpleEnoughForHeapSRA(GV, MI)) {
// If this is a fixed size array, transform the Malloc to be an alloc of
// structs. malloc [100 x struct],1 -> malloc struct, 100
Added: llvm/trunk/test/Transforms/GlobalOpt/2008-12-16-HeapSRACrash-2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/2008-12-16-HeapSRACrash-2.ll?rev=61102&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GlobalOpt/2008-12-16-HeapSRACrash-2.ll (added)
+++ llvm/trunk/test/Transforms/GlobalOpt/2008-12-16-HeapSRACrash-2.ll Tue Dec 16 15:24:51 2008
@@ -0,0 +1,24 @@
+; RUN: llvm-as < %s | opt -globalopt | llvm-dis
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
+target triple = "i386-apple-darwin7"
+ %struct.foo = type { i32, i32 }
+ at X = internal global %struct.foo* null ; <%struct.foo**> [#uses=2]
+
+define void @bar(i32 %Size) nounwind noinline {
+entry:
+ %tmp = malloc [1000000 x %struct.foo] ; <[1000000 x %struct.foo]*> [#uses=1]
+ %.sub = getelementptr [1000000 x %struct.foo]* %tmp, i32 0, i32 0 ; <%struct.foo*> [#uses=1]
+ store %struct.foo* %.sub, %struct.foo** @X, align 4
+ ret void
+}
+
+define i32 @baz() nounwind readonly noinline {
+bb1.thread:
+ %tmpLD1 = load %struct.foo** @X, align 4 ; <%struct.foo*> [#uses=2]
+ br label %bb1
+
+bb1: ; preds = %bb1, %bb1.thread
+ %tmp = phi %struct.foo* [ %tmpLD1, %bb1.thread ], [ %tmpLD1, %bb1 ] ; <%struct.foo*> [#uses=1]
+ %0 = getelementptr %struct.foo* %tmp, i32 1 ; <%struct.foo*> [#uses=0]
+ br label %bb1
+}
More information about the llvm-commits
mailing list