Fix PR12858, a crash due to GVN's PRE not fully removing an instruction from the
authorDuncan Sands <baldrick@free.fr>
Tue, 22 May 2012 14:17:53 +0000 (14:17 +0000)
committerDuncan Sands <baldrick@free.fr>
Tue, 22 May 2012 14:17:53 +0000 (14:17 +0000)
leader table.  That's because it wasn't expecting instructions to turn up as
leader for a value number that is not its own, but equality propagation could
create this situation.  One solution is to have the leader table use a WeakVH
but this slows down GVN by about 5%.  Instead just have equality propagation not
add instructions to the leader table, only constants and arguments.  In theory
this might cause GVN to run more (each time it changes something it runs again)
but it doesn't seem to occur enough to cause a slow down.

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

lib/Transforms/Scalar/GVN.cpp
test/Transforms/GVN/2012-05-22-PreCrash.ll [new file with mode: 0644]

index 915871e3c2300229cf903574599494dec7cef510..09aff1b3b3cb4a3346b1e970d5471a5e9af30c35 100644 (file)
@@ -557,12 +557,12 @@ namespace {
     }
     
     /// removeFromLeaderTable - Scan the list of values corresponding to a given
-    /// value number, and remove the given value if encountered.
-    void removeFromLeaderTable(uint32_t N, Value *V, BasicBlock *BB) {
+    /// value number, and remove the given instruction if encountered.
+    void removeFromLeaderTable(uint32_t N, Instruction *I, BasicBlock *BB) {
       LeaderTableEntry* Prev = 0;
       LeaderTableEntry* Curr = &LeaderTable[N];
 
-      while (Curr->Val != V || Curr->BB != BB) {
+      while (Curr->Val != I || Curr->BB != BB) {
         Prev = Curr;
         Curr = Curr->Next;
       }
@@ -2021,9 +2021,15 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, BasicBlock *Root) {
             DT->properlyDominates(cast<Instruction>(RHS)->getParent(), Root)) &&
            "Instruction doesn't dominate scope!");
 
-    // If value numbering later deduces that an instruction in the scope is equal
-    // to 'LHS' then ensure it will be turned into 'RHS'.
-    addToLeaderTable(LVN, RHS, Root);
+    // If value numbering later sees that an instruction in the scope is equal
+    // to 'LHS' then ensure it will be turned into 'RHS'.  In order to preserve
+    // the invariant that instructions only occur in the leader table for their
+    // own value number (this is used by removeFromLeaderTable), do not do this
+    // if RHS is an instruction (if an instruction in the scope is morphed into
+    // LHS then it will be turned into RHS by the next GVN iteration anyway, so
+    // using the leader table is about compiling faster, not optimizing better).
+    if (!isa<Instruction>(RHS))
+      addToLeaderTable(LVN, RHS, Root);
 
     // Replace all occurrences of 'LHS' with 'RHS' everywhere in the scope.  As
     // LHS always has at least one use that is not dominated by Root, this will
diff --git a/test/Transforms/GVN/2012-05-22-PreCrash.ll b/test/Transforms/GVN/2012-05-22-PreCrash.ll
new file mode 100644 (file)
index 0000000..b488dda
--- /dev/null
@@ -0,0 +1,33 @@
+; RUN: opt < %s -gvn
+; PR12858
+
+define void @fn5(i16 signext %p1, i8 signext %p2) nounwind uwtable {
+entry:
+  br i1 undef, label %if.else, label %if.then
+
+if.then:                                          ; preds = %entry
+  br label %if.end
+
+if.else:                                          ; preds = %entry
+  %conv = sext i16 %p1 to i32
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  %conv1 = sext i16 %p1 to i32
+  br i1 undef, label %if.then3, label %if.else4
+
+if.then3:                                         ; preds = %if.end
+  br label %if.end12
+
+if.else4:                                         ; preds = %if.end
+  %conv7 = sext i8 %p2 to i32
+  %cmp8 = icmp eq i32 %conv1, %conv7
+  br i1 %cmp8, label %if.then10, label %if.end12
+
+if.then10:                                        ; preds = %if.else4
+  br label %if.end12
+
+if.end12:                                         ; preds = %if.then10, %if.else4, %if.then3
+  %conv13 = sext i8 %p2 to i32
+  ret void
+}