Commit Graph

7 Commits

Author SHA1 Message Date
Max Kazantsev 18374180e3 [MustExecute] Fix algorithmic bug in isGuaranteedToExecute. PR38514
The description of `isGuaranteedToExecute` does not correspond to its implementation.
According to description, it should return `true` if an instruction is executed under the
assumption that its loop is *entered*. However there is a sophisticated alrogithm inside
that tries to prove that the instruction is executed if the loop is *exited*, which is not the
same thing for infinite loops. There is an attempt to protect from dealing with infinite loops
by prohibiting loops without exit blocks, however an infinite loop can have exit blocks.

As result of that, MustExecute can falsely consider some blocks that are never entered as
mustexec, and LICM can hoist dangerous instructions out of them basing on this fact.
This may introduce UB to programs which did not contain it initially.

This patch removes the problematic algorithm and replaced it with a one which tries to
prove what is required in description.

Differential Revision: https://reviews.llvm.org/D50558
Reviewed By: reames


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@339984 91177308-0d34-0410-b5e6-96231b3b80d8
2018-08-17 06:19:17 +00:00
Max Kazantsev 224f18f358 [NFC] Add tests that demonstrate that MustExecute is fundamentally broken
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@339417 91177308-0d34-0410-b5e6-96231b3b80d8
2018-08-10 09:20:46 +00:00
Serguei Katkov 5f0ee1d59d [LICM] Extend the MustExecute scope
CanProveNotTakenFirstIteration utility does not handle the case when
condition of the branch is a constant. Add its handling.

Reviewers: reames, anna, mkazantsev
Reviewed By: reames
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D46996



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@332695 91177308-0d34-0410-b5e6-96231b3b80d8
2018-05-18 04:56:28 +00:00
Philip Reames 16193ef0b3 [MustExecute] Shwo the effect of using full loop info variant
Most basic possible test for the logic used by LICM.

Also contains a speculative build fix for compiles which complain about a definition of a stuct K; followed by a declaration as class K;



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@328058 91177308-0d34-0410-b5e6-96231b3b80d8
2018-03-20 23:00:54 +00:00
Philip Reames eec69847b2 [MustExecute] Add simplest possible test for LoopSafetyOnfo
(Currently showing without, will enable and check in diff to show impact)



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@328056 91177308-0d34-0410-b5e6-96231b3b80d8
2018-03-20 22:55:20 +00:00
Philip Reames 284e7ac42d [MustExecute] Use the annotation style printer
As suggested in the original review (https://reviews.llvm.org/D44524), use an annotation style printer instead.

Note: The switch from -analyze to -disable-output in tests was driven by the fact that seems to be the idiomatic style used in annoation passes.  I tried to keep both working, but the old style pass API for printers really doesn't make this easy.  It invokes (runOnFunction, print(Module)) repeatedly.  I decided the extra state wasn't worth it given the old pass manager is going away soonish anyway.



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@328015 91177308-0d34-0410-b5e6-96231b3b80d8
2018-03-20 18:43:44 +00:00
Philip Reames 4961d4cd76 Add an analysis printer for must execute reasoning
Many of our loop passes make use of so called "must execute" or "guaranteed to execute" facts to prove the legality of code motion. The basic notion is that we know (by assumption) an instruction didn't fault at it's original location, so if the location we move it to is strictly post dominated by the original, then we can't have introduced a new fault.

At the moment, the testing for this logic is somewhat adhoc and done mostly through LICM. Since I'm working on that code, I want to improve the testing. This patch is the first step in that direction. It doesn't actually test the variant used by the loop passes - I need to move that to the Analysis library first - but instead exercises an alternate implementation used by SCEV. (I plan on merging both implementations.)

Note: I'll be replacing the printing logic within this with an annotation based version in the near future.  Anna suggested this in review, and it seems like a strictly better format.  

Differential Revision: https://reviews.llvm.org/D44524



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@328004 91177308-0d34-0410-b5e6-96231b3b80d8
2018-03-20 17:09:21 +00:00