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

Weiming Zhao via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 9 14:17:29 PST 2015


weimingz added inline comments.

================
Comment at: lib/Transforms/Scalar/GVN.cpp:646
@@ +645,3 @@
+        addToLeaderTable(Num, I, I->getParent());
+    }
+
----------------
dberlin wrote:
> 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)
> 
> 
> 
Is it possible that the new instruction has some existing GVN ? I was thinking calling processInstruction, but concerned that it may have side effect.



http://reviews.llvm.org/D14479





More information about the llvm-commits mailing list