[llvm-commits] [llvm] r157251 - in /llvm/trunk: lib/Transforms/Scalar/GVN.cpp test/Transforms/GVN/2012-05-22-PreCrash.ll

[Duncan Sands]

Author: baldrick
Date: Tue May 22 09:17:53 2012
New Revision: 157251

URL: http://llvm.org/viewvc/llvm-project?rev=157251&view=rev
Log:
Fix PR12858, a crash due to GVN's PRE not fully removing an instruction from the
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.

Added:
    llvm/trunk/test/Transforms/GVN/2012-05-22-PreCrash.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=157251&r1=157250&r2=157251&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Tue May 22 09:17:53 2012
@@ -557,12 +557,12 @@
     }
     
     /// 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 @@
             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

Added: llvm/trunk/test/Transforms/GVN/2012-05-22-PreCrash.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/2012-05-22-PreCrash.ll?rev=157251&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/2012-05-22-PreCrash.ll (added)
+++ llvm/trunk/test/Transforms/GVN/2012-05-22-PreCrash.ll Tue May 22 09:17:53 2012
@@ -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
+}