Commit Graph

958 Commits

Author SHA1 Message Date
Nikita Popov f00cd27646 [Verifier] Verify llvm.access.group metadata
According to LangRef, an access scope must have zero operands and
be distinct. The access group may either be a single access scope
or a list of access scopes.

LoopInfo may assert if this is not the case.
2022-03-14 16:16:36 +01:00
Arthur Eubanks 053c2a0020 [SimplifyCFG][OpaquePtr] Check store type when merging conditional store 2022-02-20 11:29:54 -08:00
Roman Lebedev 371fcb720e
[SimplifyCFG][PhaseOrdering] Defer lowering switch into an integer range comparison and branch until after at least the IPSCCP
That transformation is lossy, as discussed in
https://github.com/llvm/llvm-project/issues/53853
and https://github.com/rust-lang/rust/issues/85133#issuecomment-904185574

This is an alternative to D119839,
which would add a limited IPSCCP into SimplifyCFG.

Unlike lowering switch to lookup, we still want this transformation
to happen relatively early, but after giving a chance for the things
like CVP to do their thing. It seems like deferring it just until
the IPSCCP is enough for the tests at hand, but perhaps we need to
be more aggressive and disable it until CVP.

Fixes https://github.com/llvm/llvm-project/issues/53853
Refs. https://github.com/rust-lang/rust/issues/85133

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D119854
2022-02-17 12:13:55 +03:00
Max Kazantsev 1cd6454ade [Test] Add test demonstating how deopt bundle constants may be merged into instruction 2022-02-15 20:28:41 +07:00
Nikita Popov 8f1350e03a [IR] Check GEP source type when comparing instructions
Two GEPs with same indices but different source type are not the
same.

Worth noting that FunctionComparator already handles this correctly.
2022-02-11 12:32:04 +01:00
Roman Lebedev c8ba2b67a0
[SimplifyCFG] 'merge compatible invokes': fully support indirect invokes
As long as *all* the invokes in the set are indirect,
we can merge them, but don't merge direct invokes into the set,
even though it would be legal to do.
2022-02-08 21:29:38 +03:00
Roman Lebedev 414b47645d
[SimplifyCFG] 'merge compatible invokes': don't create trivial PHI's with all-identical incoming values 2022-02-08 21:29:38 +03:00
Roman Lebedev e2aed0b047
[NFC][SimplifyCFG] 'merge compatible invokes': tests for indirect invokes. 2022-02-08 21:29:38 +03:00
Roman Lebedev 42ca7cc889
[SimplifyCFG] 'merge compatible invokes': support normal destination w/ uses
If the original invokes had uses, the uses must have been in PHI's,
but that immediately results in the incoming values being incompatible.
But we'll replace uses of the original invokes with the use of the
merged invoke, so as long as the incoming values become compatible
after that, we can merge.
2022-02-08 17:49:38 +03:00
Roman Lebedev 9986d60224
[SimplifyCFG] 'merge compatible invokes': support normal destination w/ PHIs but no uses
As long as the incoming values for all the invokes in the set
are identical, we can merge the invokes.
2022-02-08 17:49:38 +03:00
Roman Lebedev 8411560fd0
[SimplifyCFG] 'merge compatible invokes': support normal destination w/ no uses, no PHI's
Even if the invokes have normal destination, iff it's the same block,
we can merge them. For now, require that there are no PHI nodes,
and the returned values of invokes aren't used.
2022-02-08 17:49:38 +03:00
Roman Lebedev 1d5a3f70dc
[NFC][SimplifyCFG] 'merge compatible invokes': more tests for various edge-cases 2022-02-08 17:49:38 +03:00
Roman Lebedev 18ff1ec3c3
Reland [SimplifyCFG] `markAliveBlocks()`: recognize that normal dest of `invoke`d `noreturn` function is `unreachable`
As per LangRef's definition of `noreturn` attribute:
```
noreturn
  This function attribute indicates that the function never returns
  normally, hence through a return instruction.
  This produces undefined behavior at runtime if the function
  ever does dynamically return. nnotated functions may still
  raise an exception, i.a., nounwind is not implied.
```

So if we `invoke` a `noreturn` function, and the normal destination
of an invoke is not an `unreachable`, point it at the new `unreachable`
block.

The change/fix from the original commit is that we now actually create
the new block, and don't just repurpose the original block,
because said normal destination block could have other users.

This reverts commit db1176ce66,
relanding commit 598833c987.
2022-02-05 02:58:19 +03:00
Roman Lebedev 67cb8fd75b
[NFC][SimplifyCFG] Add test w/ shared normal dest of a `noreturn` `invoke`d function 2022-02-05 02:58:19 +03:00
Roman Lebedev db1176ce66
Revert "[SimplifyCFG] `markAliveBlocks()`: recognize that normal dest of `invoke`d `noreturn` function is `unreachable`"
The normal destination may have other uses.

This reverts commit 598833c987.
2022-02-05 02:30:20 +03:00
Roman Lebedev 598833c987
[SimplifyCFG] `markAliveBlocks()`: recognize that normal dest of `invoke`d `noreturn` function is `unreachable`
As per LangRef's definition of `noreturn` attribute:
```
noreturn
  This function attribute indicates that the function never returns
  normally, hence through a return instruction.
  This produces undefined behavior at runtime if the function
  ever does dynamically return. nnotated functions may still
  raise an exception, i.a., nounwind is not implied.
```
2022-02-05 02:15:07 +03:00
Roman Lebedev 75c1d1dab4
[NFC][SimplifyCFG] Add test showing failure to drop normal dest of noreturn invoke 2022-02-05 02:15:07 +03:00
Roman Lebedev f5353c10af
[NFC][SimplifyCFG] 'merge compatible invokes': tests for non-`unreachable` normal destination 2022-02-05 02:15:07 +03:00
Roman Lebedev 55cd727c9a
[SimplifyCFG] 'merge compatible invokes': allow PHI nodes in landing pads
... iff the incoming values for the invokes-to-be-merged
are compatible (identical).
2022-02-04 20:26:44 +03:00
Roman Lebedev 332d70cd45
[NFC][SimplifyCFG] 'merge compatible invokes': tests w/ PHI's in landingpad 2022-02-04 20:26:44 +03:00
Roman Lebedev 36df803dfd
[SimplifyCFG] Merge compatible `invoke`s of a `landingpad`
While nowadays SimplifyCFG knows how to hoist code from then-else blocks,
sink code from unconditional predecessors, and even promote the latter
by tail-merging `ret`/`resume` function terminators, that isn't everything.

While i (& others) have been trying to deal with merging/sinking `unreachable`,
apparently perhaps the more impactful remaining problem is merging the `throw`
calls.

If we start at the `landingpad`, all the predecessors are unwind edges of `invoke`s,
and in some cases some of the `invoke`s are mergeable.
```
/// This is a weird mix of hoisting and sinking. Visually, it goes from:
///          [...]        [...]
///            |            |
///        [invoke0]    [invoke1]
///           / \          / \
///     [cont0] [landingpad] [cont1]
/// to:
///      [...] [...]
///          \ /
///       [invoke]
///          / \
///     [cont] [landingpad]
```

This simplifies the IR/CFG, at the cost of debug info and extra PHI nodes.

Note that we don't require for *all* the `invokes` of the `landingpad`
to be mergeable, they can form more than a single set, we gracefully handle that.

For now, i completely disallowed normal destination, PHI nodes and indirect invokes
but that can be supported.

Out of all the CTMark projects, only 7zip is C++, so there isn't much impact:
https://llvm-compile-time-tracker.com/compare.php?from=ba8eb31bd9542828f6424e15a3014f80f14522c8&to=722fc871c84f14157d45c2159bc9c8c7e2825785&stat=size-total
... but there it currently causes size-total decrease.

Differential Revision: https://reviews.llvm.org/D117805
2022-02-04 17:04:21 +03:00
Roman Lebedev 6afbf8354b
[NFC][SimplifyCFG] 'merge compatible invokes': test with PHI nodes in unreachable normal destinations 2022-02-04 16:52:09 +03:00
Nikita Popov 46f9e45ef0 [Statepoint] Update gc.statepoint calls in tests with elementtype (NFC)
This updates tests for the LangRef change in D117890.
2022-02-04 14:15:41 +01:00
Roman Lebedev ee4ba9f3a1
Revert "[SimplifyCFG] Start redesigning `FoldTwoEntryPHINode()`."
Unfortunately, it seems we really do need to take the long route;
start from the "merge" block, find (all the) "dispatch" blocks,
and deal with each "dispatch" block separately, instead of simply
starting from each "dispatch" block like it would logically make sense,
otherwise we run into a number of other missing folds around
`switch` formation, missing sinking/hoisting and phase ordering.

This reverts commit 85628ce75b.
This reverts commit c5fff90953.
This reverts commit 34a98e1046.
This reverts commit 1e353f0922.
2022-02-03 12:32:50 +03:00
Roman Lebedev 1e353f0922
[SimplifyCFG] Start redesigning `FoldTwoEntryPHINode()`.
The current `FoldTwoEntryPHINode()` is not quite designed correctly.
It starts from the merge point, and then tries to detect
the 'divergence' point.

Because of that, it is limited to the simple two-predecessor case,
where the PHI completely goes away. but that is rather pessimistic,
and it doesn't make much sense from the costmodel side of things.

For example if there is some other unrelated predecessor of
the merge point,  we could split the merge point so that
the then/else blocks first branch to an empty block
and then to the merge point, and then we'd be able to speculate
the then/else code.

But if we'd instead simply start at the divergence point,
and look for the merge point, then we'll just natively support this case.

There's also the fact that `SpeculativelyExecuteBB()` already does
just that, but only if there is a single block to speculate,
and with a much more restrictive cost model.
But that also means we have code duplication.

Now, sadly, while this is as much NFCI as possible,
there is just no way to cleanly migrate to
the proper implementation. The results *are* going to be different
somewhat because of various phase ordering effects and SimplifyCFG
block iteration strategy.
2022-02-02 17:53:56 +03:00
Roman Lebedev 73cb542930
[NFC][SimplifyCFG] Autogenerate checklines in a few tests being affected by upcoming change 2022-02-02 17:53:56 +03:00
Roman Lebedev 1455eddcf7
[NFC][SimplifyCFG] Add some tests for `invoke` merging 2022-01-20 20:37:29 +03:00
pvellien 4e1c207726 [SimplifyCFG] Fix assertion failure when reusing table switch comparison
After D116332, some icmps no longer fold with the target-independent
constant folder. The SimplifyCFG code assumed that the comparison
would always fold, which is not guaranteed. Explicitly check that the
result is either true or false.

Differential Revision: https://reviews.llvm.org/D117184
2022-01-18 09:30:54 +01:00
Roman Lebedev 82c8aca934
[SimplifyCFG] Be more aggressive when sinking into block followed by unreachable
I strongly believe we need some variant of this.

The main problem is e.g. that the glibc's assert has 4 parameters,
but the profitability check is only okay with one extra phi node,
so D116692 doesn't even trigger on most of the expected cases.

While that restriction probably makes sense in normal code, if we
are about to run off of a cliff (into an `unreachable`), this
successor block is unlikely so the cost to setup these PHI nodes
should not be on the hotpath, and shouldn't matter performance-wise.

Likewise, we don't sink if there are unconditional predecessors
UNLESS we'd sink at least one non-speculatable instruction,
which is a performance workaround, but if we are about to run into
`unreachable`, it shouldn't matter.

Note that we only allow the case where there are at
most unconditiona branches on the way to the unreachable block.

Differential Revision: https://reviews.llvm.org/D117045
2022-01-13 23:30:31 +03:00
Roman Lebedev 7b7a49a9fb
[NFC][SimplifyCFG] Add some more tests for sinking into 'unreachable' block 2022-01-11 22:35:20 +03: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
Max Kazantsev 21adcdb712 [Test] Regenerate checks using auto-update script 2021-10-28 15:13:43 +07:00
Max Kazantsev 4024ca8922 [Test] Add test showing missing simplifycfg opportunity for Phi with undef inputs 2021-10-28 11:23:07 +07:00
Max Kazantsev 31822e0530 [Test] Add test for PR52290
Demonstrates hang in iterativelySimplifyCFG.
2021-10-25 18:25:59 +07:00
Nikita Popov 1848525842 [CodeMetrics] Don't require speculatability for ephemeral values
As discussed in D112016, our current requirement of speculatability
for ephemeral is overly strict: What we really care about is that
the instruction will be DCEd once the assume is dropped. For that
it is sufficient that the instruction is side-effect free and not
a terminator.

In particular, this allows non-dereferenceable loads to be ephemeral
values.

Differential Revision: https://reviews.llvm.org/D112179
2021-10-21 20:30:01 +02:00
Nikita Popov 8e4ae603d6 [Tests] Add tests for non-speculatable ephemeral values
The loads in these examples are currently not considered ephemeral
because they are not speculatable.
2021-10-20 23:33:36 +02:00
Arthur Eubanks e7249e4acf [SimplifyCFG] Ignore free instructions when computing cost for folding branch to common dest
When determining whether to fold branches to a common destination by
merging two blocks, SimplifyCFG will count the number of instructions to
be moved into the first basic block. However, there's no reason to count
free instructions like bitcasts and other similar instructions.

This resolves missed branch foldings with -fstrict-vtable-pointers in
llvm-test-suite's lambda benchmark.

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D108837
2021-09-22 09:52:37 -07:00
Max Kazantsev 073b254cff [SimplifyCFG] Redirect switch cases that lead to UB into an unreachable block
When following a case of a switch instruction is guaranteed to lead to
UB, we can safely break these edges and redirect those cases into a newly
created unreachable block. As result, CFG will become simpler and we can
remove some of Phi inputs to make further analyzes easier.

Patch by Dmitry Bakunevich!

Differential Revision: https://reviews.llvm.org/D109428
Reviewed By: lebedev.ri
2021-09-21 10:45:19 +07:00
Owen Anderson 68079ef0eb Teach SimplifyCFG to fold switches into lookup tables in more cases.
In particular, it couldn't handle cases where lookup table constant
expressions involved bitcasts. This does not seem to come up
frequently in C++, but comes up reasonably often in Rust via
`#[derive(Debug)]`.

Originally reported by pcwalton.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D109565
2021-09-15 22:07:08 +00:00
Max Kazantsev 7e337d8ba2 [Test] Add more sophisticated tests for switch UB opt
Optimizer is being too smart with existing tests, and the transform
gets concealed by following transforms.
2021-09-13 15:28:23 +07:00
Nikita Popov 90ec6dff86 [OpaquePtr] Forbid mixing typed and opaque pointers
Currently, opaque pointers are supported in two forms: The
-force-opaque-pointers mode, where all pointers are opaque and
typed pointers do not exist. And as a simple ptr type that can
coexist with typed pointers.

This patch removes support for the mixed mode. You either get
typed pointers, or you get opaque pointers, but not both. In the
(current) default mode, using ptr is forbidden. In -opaque-pointers
mode, all pointers are opaque.

The motivation here is that the mixed mode introduces additional
issues that don't exist in fully opaque mode. D105155 is an example
of a design problem. Looking at D109259, it would probably need
additional work to support mixed mode (e.g. to generate GEPs for
typed base but opaque result). Mixed mode will also end up
inserting many casts between i8* and ptr, which would require
significant additional work to consistently avoid.

I don't think the mixed mode is particularly valuable, as it
doesn't align with our end goal. The only thing I've found it to
be moderately useful for is adding some opaque pointer tests in
between typed pointer tests, but I think we can live without that.

Differential Revision: https://reviews.llvm.org/D109290
2021-09-10 15:18:23 +02:00
Max Kazantsev 09d0fa3bbe [Test] Add tests showing missed opportunity for SimplifyCFG for switches
Patch by Dmitry Bakunevich!
2021-09-10 11:16:13 +07:00
Roman Lebedev 909cba9699
[SimplifyCFG] performBranchToCommonDestFolding(): require block-closed SSA form for bonus instructions (PR51125)
I can't seem to wrap my head around the proper fix here,
we should be fine without this requirement, iff we can form this form,
but the naive attempt (https://reviews.llvm.org/D106317) has failed.
So just to unblock the release, put up a restriction.

Fixes https://bugs.llvm.org/show_bug.cgi?id=51125
2021-09-09 12:28:09 +03:00
Max Kazantsev 29d054bf12 [SimplifyCFG] Preserve knowledge about guarding condition by adding assume
This improvement adds "assume" after removal of branch basing on UB in successor block.

Consider the following example:

```
pred:
  x = ...
  cond = x > 10
  br cond, bb, other.succ

bb:
  phi [nullptr, pred], ... // other possible preds
  load(phi) // UB if we came from pred

other.succ:
  // here we know that x <= 10, but this knowledge is lost
  // after the branch is turned to unconditional unless we
  // preserve it with assume.
```

If we remove the branch basing on knowledge about UB in a successor block,
then the fact that x <= 10 is other.succ might be lost if this condition is
not inferrable from any dominating condition. To preserve this knowledge, we
can add assume intrinsic with (possibly inverted) branch condition.

Patch by Dmitry Bakunevich!

Differential Revision: https://reviews.llvm.org/D109054
Reviewed By: lebedev.ri
2021-09-08 14:05:17 +07:00
Dávid Bolvanský 2aea581004 [NFC] Added testcase for PR48641 2021-09-04 10:44:39 +02:00
Arthur Eubanks 97ae9193df [test] Precommit test for D108837 2021-08-27 13:40:23 -07:00
Roman Lebedev 5d4f37e895
[NFCI][SimplifyCFG] Rewrite `createUnreachableSwitchDefault()`
The only thing that function should do as per it's semantic,
is to ensure that the switch's default is a block consisting only of
an `unreachable` terminator.

So let's just create such a block and update switch's default
to point to it. There should be no need for all this weird dance
around predecessors/successors.
2021-08-20 13:28:08 +03:00
Roman Lebedev 2eb554a9fe
Revert "Reland [SimplifyCFG] performBranchToCommonDestFolding(): form block-closed SSA form before cloning instructions (PR51125)"
This is still wrong, as failing bots suggest.

This reverts commit 3d9beefc7d.
2021-08-16 11:07:42 +03:00
Roman Lebedev 3d9beefc7d
Reland [SimplifyCFG] performBranchToCommonDestFolding(): form block-closed SSA form before cloning instructions (PR51125)
... with test change this time.

LLVM IR SSA form is "implicit" in `@pr51125`. While is a valid LLVM IR,
and does not require any PHI nodes, that completely breaks the further logic
in `CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses()`
that updates the live-out uses of the bonus instructions.

What i believe we need to do, is to first make the SSA form explicit,
by inserting tautological PHI nodes, and rewriting the offending uses.

```
$ /builddirs/llvm-project/build-Clang12/bin/opt -load /repositories/alive2/build-Clang-release/tv/tv.so -load-pass-plugin /repositories/alive2/build-Clang-release/tv/tv.so -tv -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 -tv -o /dev/null /tmp/test.ll

----------------------------------------
@global_pr51125 = global 4 bytes, align 4

define i32 @pr51125() {
%entry:
  br label %L

%L:
  %ld = load i32, * @global_pr51125, align 4
  %iszero = icmp eq i32 %ld, 0
  br i1 %iszero, label %exit, label %L2

%L2:
  store i32 4294967295, * @global_pr51125, align 4
  %cmp = icmp eq i32 %ld, 4294967295
  br i1 %cmp, label %L, label %exit

%exit:
  %r = phi i32 [ %ld, %L2 ], [ %ld, %L ]
  ret i32 %r
}
=>
@global_pr51125 = global 4 bytes, align 4

define i32 @pr51125() {
%entry:
  %ld.old = load i32, * @global_pr51125, align 4
  %iszero.old = icmp eq i32 %ld.old, 0
  br i1 %iszero.old, label %exit, label %L2

%L2:
  %ld2 = phi i32 [ %ld.old, %entry ], [ %ld, %L2 ]
  store i32 4294967295, * @global_pr51125, align 4
  %cmp = icmp ne i32 %ld2, 4294967295
  %ld = load i32, * @global_pr51125, align 4
  %iszero = icmp eq i32 %ld, 0
  %or.cond = select i1 %cmp, i1 1, i1 %iszero
  br i1 %or.cond, label %exit, label %L2

%exit:
  %ld1 = phi i32 [ poison, %L2 ], [ %ld.old, %entry ]
  %r = phi i32 [ %ld2, %L2 ], [ %ld.old, %entry ]
  ret i32 %r
}
Transformation seems to be correct!

```

Fixes https://bugs.llvm.org/show_bug.cgi?id=51125

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D106317
2021-08-15 19:16:04 +03:00
Roman Lebedev 77a06a9c33
[NFC][SimplifyCFG] Autogenerate check lines in a test to declutter further update 2021-08-15 19:02:32 +03:00