<div dir="ltr">I'm tempted to suggest putting this in valueComesBefore or otherwise refactoring some logic here.<div>What you've written is basically now ValueDFS_Compare just operating on a slightly different datastructure format.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 15, 2018 at 12:52 PM, Eli Friedman via Phabricator via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">efriedma added a comment.<br>
<span class=""><br>
> because values have slightly different labels<br>
<br>
</span>Not sure you're diagnosing the issue correctly; IR optimizations shouldn't care about the names of instructions/basic blocks, as far as I know.  But in any case, we need to fix this because a bad sort predicate causes undefined behavior.<br>
<br>
Can you think of any way to write a testcase for this?<br>
<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/<wbr>PredicateInfo.cpp:110<br>
<br>
 // Perform a strict weak ordering on instructions and arguments.<br>
 static bool valueComesBefore(<wbr>OrderedInstructions &OI, const Value *A,<br>
----------------<br>
Please fix this comment to note that this only works for instructions in the same BB.<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/<wbr>PredicateInfo.cpp:557<br>
<span class="">+    // dominated instructions come after the instructions that dominate them.<br>
+    const Instruction *InstA = dyn_cast_or_null<Instruction>(<wbr>A);<br>
+    const Instruction *InstB = dyn_cast_or_null<Instruction>(<wbr>B);<br>
</span>----------------<br>
There shouldn't be null values in OpSet; you can use dyn_cast instead of dyn_cast_or_null.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D48230" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D48230</a><br>
<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>