[PATCH] D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> because values have slightly different labels

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.

Can you think of any way to write a testcase for this?



================
Comment at: lib/Transforms/Utils/PredicateInfo.cpp:110
 
 // Perform a strict weak ordering on instructions and arguments.
 static bool valueComesBefore(OrderedInstructions &OI, const Value *A,
----------------
Please fix this comment to note that this only works for instructions in the same BB.


================
Comment at: lib/Transforms/Utils/PredicateInfo.cpp:557
+    // dominated instructions come after the instructions that dominate them.
+    const Instruction *InstA = dyn_cast_or_null<Instruction>(A);
+    const Instruction *InstB = dyn_cast_or_null<Instruction>(B);
----------------
There shouldn't be null values in OpSet; you can use dyn_cast instead of dyn_cast_or_null.


https://reviews.llvm.org/D48230