[PATCH] D14479: Fix bug 25440: GVN assertion after coercing loads

[Daniel Berlin via llvm-commits]

dberlin added inline comments.

================
Comment at: lib/Transforms/Scalar/GVN.cpp:646
@@ +645,3 @@
+        addToLeaderTable(Num, I, I->getParent());
+    }
+
----------------
Close, but if you are adding only the new instructions, I believe this should be unconditional.

Think of the leader table as "the list of things in the program that have a given value number".

Even if the value number existed, if this is a new instruction, it's still a new thing that has that value number, and should still end up in the leader table.

(I'd also probably name it addNewInstruction or something)




================
Comment at: lib/Transforms/Scalar/GVN.cpp:1218
@@ -1209,2 +1217,3 @@
+    gvn.addGVN(static_cast<Instruction*>(RV));
     SrcVal->replaceAllUsesWith(RV);
 
----------------
These definitely look right


http://reviews.llvm.org/D14479