[PATCH] D129370: [SimplifyCFG] Allow SimplifyCFG hoisting to skip over non-matching instructions

Momchil Velikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 12 03:02:41 PDT 2022


chill marked an inline comment as done.
chill added a comment.

In D129370#3638912 <https://reviews.llvm.org/D129370#3638912>, @nikic wrote:

> I think you need to be more careful about the side effects here. You generally need to distinguish control flow effects from memory effects. For example, if you skip over an instruction that is not must-exec (i.e. not nothrow or not willreturn), and then want to hoist a udiv (which is side-effect free, but not generally speculatable), then that is illegal and I don't think you currently handle this correctly.

Thanks for the review, I think I have fixed those issues.

> On a more general note, this skipping of non-identical instructions in lock-step seems kind of ... random? It isn't obvious to me why we would typically expect there to be sequences of different instructions that just happen to have the same length.

Indeed, it relies on ... luck, but that applies to the original function as well, isn't it ?

And it turns out, it happens: when compiling llvm-test-suite, the `HoistThenElseCodeToIf`  triggers 3206 times, of these
722 times at least one instruction pair was hoisted over at least one non-matching pair (stats collected as in https://reviews.llvm.org/D129551)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129370/new/

https://reviews.llvm.org/D129370



More information about the llvm-commits mailing list