This is a follow-up to b71edfaa4ec3c998aadb35255ce2f60bba2940b0
since I forgot the lit.local.cfg files in that one.
Reformatting is done with `black`.
If you end up having problems merging this commit because you
have made changes to a python file, the best way to handle that
is to run git checkout --ours <yourfile> and then reformat it
with black.
If you run into any problems, post to discourse about it and
we will try to help.
RFC Thread below:
https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style
Reviewed By: barannikov88, kwk
Differential Revision: https://reviews.llvm.org/D150762
Fixes https://github.com/llvm/llvm-project/issues/62715
If a select's condition is a trivial select:
```
%s = select %cond, i1 true, i1 false
```
Unswitch on %cond, rather than %s. This fixes crashes where there is a
disparity in finding candidates and and the transformation logic.
The old LoopUnswitch pass unswitched selects, but the changes were
never ported to the new SimpleLoopUnswitch.
We unswitch by turning:
``` S = select %cond, %a, %b ```
into:
``` head: br %cond, label %then, label %tail
then: br label %tail
tail: S = phi [ %a, %then ], [ %b, %head ] ```
Unswitch selects are always nontrivial, since the successors do not
exit the loop and the loop body always needs to be cloned.
Unswitch selects always need to freeze the conditional if the
conditional could be poison or undef. Selects don't propagate
poison/undef, and branches on poison/undef causes UB.
Reland 1 - Fix the insertion of freeze instructions. The original
implementation inserts a dead freeze instruction that is not used by the
unswitched branch.
Reland 2 - Include https://reviews.llvm.org/D149560 in the same patch,
which was originally reverted along with this patch. The patch prevents
unswitching of selects with a vector conditional. This could have been
caught in SimpleLoopUnswitch/crash.ll if it included tests for
nontrivial unswitching. This reland also adds a run for the test file
with nontrivial unswitching.
Reviewed By: nikic, kachkov98, vitalybuka
Differential Revision: https://reviews.llvm.org/D138526
The old LoopUnswitch pass unswitched selects, but the changes were never
ported to the new SimpleLoopUnswitch.
We unswitch by turning:
```
S = select %cond, %a, %b
```
into:
```
head:
br %cond, label %then, label %tail
then:
br label %tail
tail:
S = phi [ %a, %then ], [ %b, %head ]
```
Unswitch selects are always nontrivial, since the successors do not exit
the loop and the loop body always needs to be cloned.
Unswitch selects always need to freeze the conditional if the
conditional could be poison or undef. Selects don't propagate
poison/undef, and branches on poison/undef causes UB.
Reviewed By: nikic, kachkov98, vitalybuka
Differential Revision: https://reviews.llvm.org/D138526
Otherwise we could produce `br <2x i1>` which are of course not legal.
```
Branch condition is not 'i1' type!
br <2 x i1> %cond.fr1, label %entry.split.us, label %entry.split
%cond.fr1 = freeze <2 x i1> %cond
LLVM ERROR: Broken module found, compilation aborted!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: /home/vchuravy/builds/llvm/bin/opt -passes=simple-loop-unswitch<nontrivial> -S
```
Fixes change introduced by https://reviews.llvm.org/D138526
Reviewed By: caojoshua
Differential Revision: https://reviews.llvm.org/D149560
The old LoopUnswitch pass unswitched selects, but the changes were never
ported to the new SimpleLoopUnswitch.
We unswitch by turning:
```
S = select %cond, %a, %b
```
into:
```
head:
br %cond, label %then, label %tail
then:
br label %tail
tail:
S = phi [ %a, %then ], [ %b, %head ]
```
Unswitch selects are always nontrivial, since the successors do not exit
the loop and the loop body always needs to be cloned.
Differential Revision: https://reviews.llvm.org/D138526
Co-authored-by: Sergey Kachkov <sergey.kachkov@syntacore.com>
This patch is making sure that we use getTopMostExitingLoop when
finding out which loops to forget, when dealing with
unswitchNontrivialInvariants and unswitchTrivialSwitch. It seems
to at least be needed for unswitchNontrivialInvariants as detected
by the included test case.
Note that unswitchTrivialBranch already used getTopMostExitingLoop.
This was done in commit 4a9cde5a791cd49b96993e6. The commit
message in that commit says "If the patch makes sense, I will also
update those places to a similar approach ...", referring to these
functions mentioned above. As far as I can tell that never happened,
but this is an attempt to finally fix that.
Fixes https://github.com/llvm/llvm-project/issues/61080
Differential Revision: https://reviews.llvm.org/D147058
When doing a trivial unswitch of a switch statement the code need
to "invalidate SCEVs for the outermost loop reached by any of the
exits", as indicated by code comments.
Depending on if we find such an outermost loop or not we can limit
the invalidation to some sub-loops or the full loop-nest. As shown
in the added test case there seem to have been some bugs in the code
that was finding the "outermost loop", so we could end up invalidating
too few loops.
Seems like commit 1bf8ae17f5e2714c8c87978 introduced the bug by
moving the code that invalidates the loops above some of the code
that computed 'OuterL'. This patch fixes that by also moving that
computation of 'OuterL' so that we compute 'OuterL' properly before
we use it for the SCEV invalidation.
Differential Revision: https://reviews.llvm.org/D146963
This corresponds to its description in LangRef:
The intrinsic @llvm.experimental.widenable.condition() returns either true or false.
Differential Revision: https://reviews.llvm.org/D146508
Reviewed By: skatkov, nikic
This fixes a compile time issue due to guarding loop unswitching based
on whether the enclosing function is cold. That approach is very
inefficient in the case of large cold functions that contain numerous
loops, since the loop pass calls isFunctionColdInCallGraph once per
loop, and that function walks all BBs in the function (twice for Sample
PGO) looking for any non-cold blocks.
Originally, this code only checked if the current Loop's header was cold
(D129599). However, that apparently caused a slowdown on a SPEC
benchmark, and the example given was that of a cold inner loop nested in
a non-cold outer loop (see comments in D129599). The fix was to check if
the whole function is cold, done in D133275.
This is overkill, and we can simply check if the header of any loop in
the current loop's loop nest is non-cold (looking at both outer and
inner loops). This patch drops the compile time for a large module by
40% with this approach.
I also updated PGO-nontrivial-unswitch2.ll since it only had one cold
loop in a non-cold function, so that it instead had IR based off the
example given in the comments relating to the SPEC degradation in
D129599. I confirmed that the new version of the test fails with the
original check done in D129599 of only the current loop's header
coldness.
Similarly updated test PGO-nontrivial-unswitch.ll to contain a cold loop
in a cold loop nest, and created PGO-nontrivial-unswitch3.ll to contain
a non-cold loop in a non-cold loop nest.
Differential Revision: https://reviews.llvm.org/D146383
Reuse StructuralHash and allow it to be used in non-expensive checks builds.
Move PreservedAnalysisChecker further down StandardInstrumentations so other Instrumentations (e.g. printing) have a chance to run before PreservedAnalysisChecker crashes.
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D146003
Since canonicalizeForInvariantConditionInjection is introduced the
in loop successor may be the second successor.
Reviewed By: mkazantsev
Differential Revision: https://reviews.llvm.org/D144361
When loop condition isn't immediately in the form supported by invariant injection
unswitching, try to canonicalize it to this form.
Differential Revision: https://reviews.llvm.org/D143175
Reviewed By: skatkov
Name 'len' implicitly hints that it should be non-negative, but in fact in some of
these tests the baseline profitable scenario for the transform to be profitable is
when this thing is negative. Rename to avoid this confusion.
When branch is so hot that sum of its frequencies overflows, we have an assertion
failure when trying to construct BranchProbability. Such cases are not interesting
as candidate for unswitching anyways (their branches are too hot), so reject them.
The transform and all related updates don't expect the situation when candidate
is from an inner loop. I think we *might* still do something in this case, but
the current implementation doesn't expect this and does incorrect loop info
updates in this situation.
Details: https://github.com/llvm/llvm-project/issues/60736
Based on https://discourse.llvm.org/t/rfc-inject-invariant-conditions-to-loops-to-enable-unswitching-and-constraint-elimination
This transform attempts to handle the following loop:
```
for (...) {
x = <some variant>
if (x <u C1) {} else break;
if (x <u C2) {} else break;
}
```
Here `x` is some loop-variant value, and `C1` and `C2` are loop invariants.
As we see, this loop has no invariant checks we can unswitch on. However, there is an
invariant condition that can make the second check redundant. Specifically, it is `C1 <=u C2`.
We can modify this code in the following way:
```
for (...) {
x = <some variant>
if (x <u C1) {} else break;
if (C1 <=u C2) {
/* no check is required */
}
else {
// do the check normally
if (x <u C2) {} else break;
}
}
```
Now we have an invariant condition `C1 <=u C2` and can unswitch on it.
This patch introduces the basic version of this transform, with some limitations,
all of them seem liftable (but needs more work & testing):
- All checks are `ult` condition;
- All branches in question stay in loop if the said condition is true and leave it otherwise;
- All in-loop branches are hot enough;
There is also a room for improvement cost model. So far we evalutate the cost of
unswitching this newly injected invariant branch the same as if we would unswitch
on 2nd condition, which is not exactly precise (but also not grossly wrong).
Differential Revision: https://reviews.llvm.org/D136233
Reviewed By: skatkov
For internal functions, globals-aa returns different ModRefInfo
results if they are inlined and are no longer called by external
functions. This causes an assertion failure when cloning memoryssa
accesses.
Fixes: https://github.com/llvm/llvm-project/issues/59546
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D141185
A few more that I missed in commit 3528e63d89305907b3d6e.
There could be more duplicates remaining, since I've only focused
on exactly duplicated "RUN: opt" lines (ignoring multi line RUN
lines ending with '\').
If globals-aa is enabled, because of the deletion of memory instructions, there
may be call instruction that is not in ModOrRefSet but is a MemoryUseOrDef.
This causes the crash in the process of cloning uses and defs.
Fixes https://github.com/llvm/llvm-project/issues/58719
Reviewed By: asbirlea
Differential Revision: https://reviews.llvm.org/D137553
Forget SCEVs based on exit phis in case SCEV looked through the phi.
After unswitching, it may not be possible to look through the phi due to
it having multiple incoming values, so it needs to be re-computed.
Fixes#58868
Unswitching adjusts the CFG in ways that may invalidate cached loop
dispositions. Clear all cached block and loop dispositions during
trivial unswitching. The same is already done for non-trivial
unswitching.
Fixes#58751.
SimpleLoopUnswitch may remove blocks from loops. Clear block and loop
dispositions in that case, to clean up invalid entries in the cache.
Fixes#58158.
Fixes#58159.
This simplifies the test case added in e399dd601 to only require indvars
and simple-loop-unswitch. This allows adding the test case for #58158 to
the same file, keeping related tests together.
This ensures PreservedCFGCheckerAnalysis is always added, independent of
whether opt was built with assertions enabled or not.
This fixes a few buildbot failures for bots that don't have assertions
enabled.
At the moment, LoopAccessAnalysis is a loop analysis for the new pass
manager. The issue with that is that LAI caches SCEV expressions and
modifications in a loop may impact SCEV expressions in other loops, but
we do not have a convenient way to invalidate LAI for other loops
withing a loop pipeline.
To avoid this issue, turn it into a function analysis which returns a
manager object that keeps track of the individual LAI objects per loop.
Fixes#50940.
Fixes#51669.
Reviewed By: aeubanks
Differential Revision: https://reviews.llvm.org/D134606
Summary:
The code for generating a name for loops for various reporting scenarios
created a name by serializing the loop into a string. This may result in
a very large name for a loop containing many blocks. Use the getName()
function on the loop instead.
Author: Jamie Schmeiser <schmeise@ca.ibm.com>
Reviewed By: Whitney (Whitney Tsang), aeubanks (Arthur Eubanks)
Differential Revision: https://reviews.llvm.org/D133587
In the current main branch, all cold loops will not be applied non-trivial unswitch. As reported in D129599, skipping these cold loops will incur regression in SPEC benchmark.
Thus, instead of skipping cold loops, now only skipping loops in cold functions.
Reviewed By: alexgatea, aeubanks
Differential Revision: https://reviews.llvm.org/D133275
With profile data, non-trivial LoopUnswitch will only apply on non-cold loops, as unswitching cold loops may not gain much benefit but significantly increase the code size.
Reviewed By: aeubanks, asbirlea
Differential Revision: https://reviews.llvm.org/D129599
