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

[Daniel Berlin via llvm-commits]

I'm tempted to suggest putting this in valueComesBefore or otherwise
refactoring some logic here.
What you've written is basically now ValueDFS_Compare just operating on a
slightly different datastructure format.


On Fri, Jun 15, 2018 at 12:52 PM, Eli Friedman via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180615/81fdc669/attachment.html>