Commit Graph

115 Commits

Author SHA1 Message Date
Roman Lebedev 7850ab2112
[NFC] Port an assortment of tests that invoke SROA to new pass manager 2022-12-01 21:17:18 +03:00
luxufan 3053a323c7 [MemorySSA] Relax assert condition in createDefinedAccess
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
2022-11-17 23:43:20 +08:00
Florian Hahn 7854a1abfd
[SimpleLoopUnswitch] Forget SCEVs for replaced phis.
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
2022-11-13 17:38:39 +00:00
Dmitry Makogon ebac59999f [SimpleLoopUnswitch] Skip trivial selects in guards conditions unswitch candidates
We do this for conditional branches, but not for guards for some reason.

Fixes pr58666.

Differential Revision: https://reviews.llvm.org/D137249
2022-11-08 13:29:27 +07:00
Florian Hahn a41cb8bf58
[SimpleLoopUnswitch] Forget block & loop dispos during trivial unswitch.
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.
2022-11-05 16:56:06 +00:00
Dmitry Makogon dd927f47e7 [Test] Add test exposing crash in SimpleLoopUnswitch
The test crashes with an assert in unswitchNontrivialInvariants.
2022-11-02 20:01:44 +07:00
Florian Hahn 1f1fb208da
[SimpleLoopUnswitch] Forget block and loop dispositions.
Also invalidate block and loop dispositions during non-trivial
unswitching.

Fixes #58564.
2022-10-30 20:44:22 +00:00
Arthur Eubanks f3a928e233 [opt] Don't translate legacy -analysis flag to require<analysis>
Tests relying on this should explicitly use -passes='require<analysis>,foo'.
2022-10-07 14:54:34 -07:00
Florian Hahn 469f0fc6a6
[SimpleLoopUnswitch] Clear dispos in deleteDeadBlocksFromLoop.
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.
2022-10-05 10:28:15 +01:00
Florian Hahn eb975cae4b
[SimpleLoopUnswitch] Simplify test, reduce the passes to trigger crash.
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.
2022-10-05 10:19:55 +01:00
Florian Hahn e399dd601f
[SimpleLoopUnswitch] Clear block and loop dispos after destroying loop.
SimpleLoopUnswitch may remove loops. Clear block and loop dispositions,
to clean up invalid entries in the cache.

Fixes #58136.
2022-10-04 10:27:52 +01:00
Florian Hahn 05b0b48b50
[SimpleLoopUnswitch] Pass -verify-cfg-preserved to test.
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.
2022-10-01 17:19:02 +01:00
Florian Hahn 7c0ff64b0f
[LAA] Change to function analysis for new PM.
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
2022-10-01 15:44:27 +01:00
Jamie Schmeiser 5e3ac79690 Loop names used in reporting can grow very large
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
2022-09-09 13:45:14 -04:00
Ruobing Han fb45f3c948 [SimpleLoopUnswitch] Skip non-trivial unswitching of cold functions
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
2022-09-06 19:13:31 -04:00
Ruobing Han 07341e3b37 [test] pre-submission for the following SimpleLoopUnswitch update 2022-09-04 11:49:51 -04:00
Ruobing Han f756f06cc4 [SimpleLoopUnswitch] Skip non-trivial unswitching of cold loops
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
2022-08-08 18:12:04 +00:00
Ruobing Han b5244fb71c [test][SimpleLoopUnswitch] Precommit test for D129599 2022-08-05 20:17:26 +00:00
Nikita Popov 534b9246a2 [LoopInfo] Allow cloning of callbr
After D129288, callbr is safe to clone without special handling.
This permits optimizations like loop unroll and loop unswitch on
loops containing callbrs.

Fixes https://github.com/llvm/llvm-project/issues/41834.

Differential Revision: https://reviews.llvm.org/D129993
2022-07-19 09:57:28 +02:00
Nikita Popov 46d5615742 [SimpleLoopUnswitch] Regenerate test checks (NFC) 2022-07-18 11:41:08 +02:00
Nikita Popov 2a721374ae [IR] Don't use blockaddresses as callbr arguments
Following some recent discussions, this changes the representation
of callbrs in IR. The current blockaddress arguments are replaced
with `!` label constraints that refer directly to callbr indirect
destinations:

    ; Before:
    %res = callbr i8* asm "", "=r,r,i"(i8* %x, i8* blockaddress(@test8, %foo))
    to label %asm.fallthrough [label %foo]
    ; After:
    %res = callbr i8* asm "", "=r,r,!i"(i8* %x)
    to label %asm.fallthrough [label %foo]

The benefit of this is that we can easily update the successors of
a callbr, without having to worry about also updating blockaddress
references. This should allow us to remove some limitations:

* Allow unrolling/peeling/rotation of callbr, or any other
  clone-based optimizations
  (https://github.com/llvm/llvm-project/issues/41834)
* Allow duplicate successors
  (https://github.com/llvm/llvm-project/issues/45248)

This is just the IR representation change though, I will follow up
with patches to remove limtations in various transformation passes
that are no longer needed.

Differential Revision: https://reviews.llvm.org/D129288
2022-07-15 10:18:17 +02:00
Florian Hahn f96aa493f0
[SimpleLoopUnswitch] Always skip trivial select and set condition.
When updating the branch instruction outside the loopduring non-trivial
 unswitching, always skip trivial selects and update the condition.

Otherwise we might create invalid IR, because the trivial select is
inside the loop, while the condition is outside the loop.

Fixes #55697.
2022-05-26 09:46:24 +01:00
Florian Hahn 32d6ef36d6
[SimpleLoopUnswitch] Skip trivial selects during trivial unswitching.
Update the remaining places in unswitchTrivialBranch to properly skip
trivial selects.

Fixes #55526.
2022-05-19 17:01:13 +01:00
Florian Hahn 41e142fdc7
Recommit "[SimpleLoopUnswitch] Collect either logical ANDs/ORs but not both."
This reverts commit 7211d5ce07.

This version fixes a crash that caused buildbot failures with the first
version.
2022-05-09 13:49:12 +01:00
Florian Hahn 4c569ceeaa
[SimpleLoopUnswitch] Add test case for crash with db7a87ed4f. 2022-05-09 13:48:56 +01:00
Florian Hahn 7211d5ce07
Revert "[SimpleLoopUnswitch] Collect either logical ANDs/ORs but not both."
This reverts commit db7a87ed4f.

This seems to cause a PPC buildbot failure:
https://lab.llvm.org/buildbot#builders/93/builds/8787
2022-05-06 22:38:15 +01:00
Florian Hahn db7a87ed4f
[SimpleLoopUnswitch] Collect either logical ANDs/ORs but not both.
After D97756, collectHomogenousInstGraphLoopInvariants may collect
conditions for both logical ANDs and logical ORs in case the root is a
select that matches both logical AND & OR.

This means the function won't return invariant values of either AND/OR
chains, but both. This can result in incorrect transformations.

See llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch-logical-and-or.ll.
Without the patch, Alive2 rejects the modified tests with:
    Source and target don't have the same return domain.

Note that this also applies to the test case added in D97756
(@test_partial_condition_unswitch_or_select). We can't unswitch on
%cond6, because the graph leading to it contains and AND and an OR.

This only fixes trivial unswitching for now, but a similar problem
likely exists with non-trivial unswitching.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D124526
2022-05-06 09:50:03 +01:00
Florian Hahn 6bd2b70877
[SimpleLoopUnswitch] Add freeze if branch execs for partial unswitching.
We cannot skip the freezing the condition if the unswitched branch
executes, if the condition is a chain of ANDs/ORs. For example, if if we
have an AND %c1, %c2 with  %c1 == undef and %c2 == 0, there would be no
branch on undef in the original code, but a branch on undef if we
unswitch %c1.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D124603
2022-05-05 09:44:07 +01:00
Florian Hahn 5387a38c38
[SimpleLoopUnswitch] Freeze individual OR/AND operands.
In some cases, it is not enough to freeze the final AND/OR operation
when chaining a number of invariant conditions together.

After creating a chain of ANDs/ORs, we assume all unswitched operands to
be either true or false. But if any of the operands is poison, the rest
of the operands could have any value after branching on the frozen
condition.

To avoid that, freeze individual operands, if needed. In some cases this
may lead to unnecessary freezes, but it seems required at least for some
cases (see trivial-unswitch-freeze-individual-conditions.ll)

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D124554
2022-05-01 20:11:05 +01:00
Florian Hahn 8b022f87b0
[SimpleLoopUnswitch] Freeze trivial conditions if needed.
Trivial unswitching can also introduce new branches on undef/poison.
Freeze the conditions if needed.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D124549
2022-04-30 19:53:36 +01:00
Florian Hahn 25d53aa9a2
[SimpleLoopUnswitch] Fix CHECK lines of tests added in 93182c9ff4.
When generating the tests I accidentally had some other changes applied.
Fix the CHECK lines for current main.
2022-04-27 21:36:53 +01:00
Florian Hahn 93182c9ff4
[SimpleLoopUnswitch] Add test cases where freeze of OR/AND cons needed.
Freezing just the result of the AND/OR here is not enough.
2022-04-27 21:19:14 +01:00
Florian Hahn 51be0925b4
[SimpleLoopUnswitch] Add trivial unswitching tests with selects.
Add tests with selects that match both logical AND and logical OR. Note
that some of the tests get miscompiled at the moment.

Also moves a related test to the newly added test file.
2022-04-27 14:06:36 +01:00
Florian Hahn 6a6cc5542b
[SimpleLoopUnswitch] Enable freezing of conditions by default.
This fixes a series of mis-compiles by SimpleLoopUnswitch.

My measurements showed no performance regression with -O3 on AArch64
in SPEC2006, SPEC2017 and a set of internal benchmarks.

Fixes #50387, #50430

Depends on D124251.

Reviewed By: nikic, aqjune

Differential Revision: https://reviews.llvm.org/D124252
2022-04-25 14:26:41 +01:00
Florian Hahn 2fee8dd621
[SimpleLoopUnswitch] Run LICM for nested unswitching tests.
When enabling freeze-loop-unswitch-cond the inserted freeze instruction
may block unswitching of parent loops if they get inserted in a block in
the parent loop (as the llvm::Loop-based invariance check only checks
 whether an instruction is in a loop block or not).

In the optimization pipeline, LICM is responsible to hoist out loop
invariant instructions to enable further unswitching. Also run LICM for
nested unswitching tests in preparation for flipping the default of
freeze-loop-unswitch-cond.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D124251
2022-04-25 12:49:08 +01:00
Florian Hahn e0f7db779e
[SimpleLoopUnswitch] Use SLU version of memoryssa-threshold option.
Update the test to use the SLU version of the option.
2022-04-23 14:03:32 +01:00
Florian Hahn b341c44010
[SimpleLoopUnswitch] Check if freeze is needed for partial unswitching.
We only need to insert a Freeze instruction if any of the conditions
may be poison. Similar checks are already done in the other places
SimpleLoopUnswitch creates Freeze instruction.

Reviewed By: aeubanks, efriedma

Differential Revision: https://reviews.llvm.org/D124259
2022-04-22 21:24:55 +01:00
Florian Hahn 1fa0bb8a80
[SimpleLoopUnswitch] Add test where all conds are guaranteed non poison.
Extra test for D124259.
2022-04-22 18:09:05 +01:00
Nick Desaulniers 79ebc3b0dd [llvm][test] rewrite callbr to use i rather than X constraint NFC
In D115311, we're looking to modify clang to emit i constraints rather
than X constraints for callbr's indirect destinations. Prior to doing
so, update all of the existing tests in llvm/ to match.

Reviewed By: void, jyknight

Differential Revision: https://reviews.llvm.org/D115410
2022-01-11 11:31:08 -08:00
hyeongyu kim 0aeb37324d [SimpleLoopUnswitch] Re-fix introduction of UB when hoisted condition may be undef or poison
https://bugs.llvm.org/show_bug.cgi?id=27506
https://bugs.llvm.org/show_bug.cgi?id=31652
https://bugs.llvm.org/show_bug.cgi?id=51043

Problems with SimpleLoopUnswitch cause the bug reports above.

```
while (...) {
  if (C) { A }
  else   { B }
}
Into:

C' = freeze(C)
if (C') {
  while (...) { A }
} else {
  while (...) { B }
}
```
This problem can be solved by adding a freeze on hoisted branches(above transform) and has been solved by D29015.
However, D29015 is now reverted by performance regression(2b5a897651)

It is not the first time that an added freeze has caused performance regression.
SimplifyCFG also had a problem with UB caused by branching-on-undef, which was solved by adding freeze to the branching condition. (D104569)
Performance regression occurred in D104569, and patches such as D105344 and D105392 were written to minimize it.

This patch will correct the SimpleLoopUnswitch as D104569 handles the SimplyCFG while minimizing performance loss by introducing patches like D105344 and D105392(This patch was rebased with the author's permission)

Reviewed By: reames

Differential Revision: https://reviews.llvm.org/D106041
2021-10-12 01:02:09 +09:00
Daniil Suchkov 45bd8d9477 [SimpleLoopUnswitch] Don't unswitch constant conditions
Added an additional check for constants after simplification of
"select _, true, false" pattern. We need to prevent attempts to unswitch constant
conditions for two reasons:
a) Doing that doesn't make any sense, in the best case it will just burn
some compile time.
b) SimpleLoopUnswitch isn't designed to unswitch constant conditions
(due to (a)), so attempting that can cause miscompiles. The attached
testcase is an example of such miscompile.

Also added an assertion that'll make sure we aren't trying to replace
constants, so it will help us prevent such bugs in future. The assertion
from D110751 is another layer of protection against such cases.

Reviewed By: aeubanks
Differential Revision: https://reviews.llvm.org/D110752
2021-10-01 21:30:54 +00:00
Daniil Suchkov bdd52e8bae [Test] Add a test exposing a miscompile in SimpleLoopUnswitch.
The miscompile was introduced by 6b4b1dc6ec.
2021-10-01 21:30:54 +00:00
Bjorn Pettersson 0f0344dd1e [SimpleLoopUnswitch] Inform pass manager when child loops are deleted
As part of the nontrivial unswitching we could end up removing child
loops. This patch add a notification to the pass manager when
that happens (using the markLoopAsDeleted callback).

Without this there could be stale LoopAccessAnalysis results cached
in the analysis manager. Those analysis results are cached based on
a Loop* as key. Since the BumpPtrAllocator used to allocate
Loop objects could be resetted between different runs of for
example the loop-distribute pass (running on different functions),
a new Loop object could be created using the same Loop pointer.
And then when requiring the LoopAccessAnalysis for the loop we
got the stale (corrupt) result from the destroyed loop.

Reviewed By: aeubanks

Differential Revision: https://reviews.llvm.org/D109257
2021-09-04 17:54:39 +02:00
Nikita Popov e11354c0a4 [Tests] Remove explicit -enable-mssa-loop-dependency options (NFC)
This is enabled by default. Drop explicit uses in preparation for
removing the option.

Also drop RUN lines that are now the same (typically modulo a
-verify-memoryssa option).
2021-08-14 21:21:07 +02:00
Arthur Eubanks 5366de7375 [SimpleLoopUnswitch] Don't non-trivially unswitch loops with catchswitch exits
SplitBlock() can't handle catchswitch.

Fixes PR50973.

Reviewed By: aheejin

Differential Revision: https://reviews.llvm.org/D105672
2021-07-14 14:07:28 -07:00
Arthur Eubanks 0024ec59a0 [NewPM][SimpleLoopUnswitch] Add option to not trivially unswitch
To help with debugging non-trivial unswitching issues.

Don't care about the legacy pass, nobody is using it.

If a pass's string params are empty (e.g. "simple-loop-unswitch"), don't
default to the empty constructor for the pass params. We should still
let the parser take care of it in case the parser has its own defaults.

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D105933
2021-07-13 16:09:42 -07:00
Bjorn Pettersson 1db2551cc1 [NewPM] Rename 'unswitch' to 'simple-loop-unswitch' in PassRegistry
It is confusing to have two ways of specifying the same pass
('simple-loop-unswitch' and 'unswitch'). This patch replaces
'unswitch' by 'simple-loop-unswitch' to get a unique identifier.

Using 'simple-loop-unswitch' instead of 'unswitch' also has the
advantage of matching how the pass is named in DEBUG_TYPE etc. So
this makes it a bit more consistent how we refer to the pass in
options such as -passes, -print-after and -debug-only.

Reviewed By: aeubanks

Differential Revision: https://reviews.llvm.org/D105628
2021-07-09 09:47:33 +02:00
Jingu Kang 873ff5a728 [SimpleLoopUnswich] Fixa a bug on ComputeUnswitchedCost with partial unswitch
There was a bug from cost calculation for partially invariant unswitch.

The costs of non-duplicated blocks are substracted from the total LoopCost, so
anything that is duplicated should not be counted.

Differential Revision: https://reviews.llvm.org/D103816
2021-06-22 16:18:00 +01:00
Jingu Kang f3a27511c9 [SimpleLoopUnswitch] Port partially invariant unswitch from LoopUnswitch to SimpleLoopUnswitch
This re-enables commit 107d19eb01 with bug fixes.

Differential Revision: https://reviews.llvm.org/D99354
2021-06-02 10:58:22 +01:00
Jingu Kang 107d19eb01 Revert "[SimpleLoopUnswitch] Port partially invariant unswitch from LoopUnswitch to SimpleLoopUnswitch"
This reverts commit 88b259c014.

It needs to fix below bugs.

https://bugs.llvm.org/show_bug.cgi?id=50279
https://bugs.llvm.org/show_bug.cgi?id=50302
2021-05-13 08:40:49 +01:00