Summary:
Here we try to avoid issues with "explicit branch" with SimplifyBranchOnICmpChain
which can check on undef. Msan by design reports branches on uninitialized
memory and undefs, so we have false report here.
In general msan does not like when we convert
```
// If at least one of them is true we can MSAN is ok if another is undefs
if (a || b)
return;
```
into
```
// If 'a' is undef MSAN will complain even if 'b' is true
if (a)
return;
if (b)
return;
```
Example
Before optimization we had something like this:
```
while (true) {
bool maybe_undef = doStuff();
while (true) {
char c = getChar();
if (c != 10 && c != 13)
continue
break;
}
// we know that c == 10 || c == 13 if we get here,
// so msan know that branch is not affected by maybe_undef
if (maybe_undef || c == 10 || c == 13)
continue;
return;
}
```
SimplifyBranchOnICmpChain will convert that into
```
while (true) {
bool maybe_undef = doStuff();
while (true) {
char c = getChar();
if (c != 10 && c != 13)
continue;
break;
}
// however msan will complain here:
if (maybe_undef)
continue;
// we know that c == 10 || c == 13, so either way we will get continue
switch(c) {
case 10: continue;
case 13: continue;
}
return;
}
```
Reviewers: eugenis, efriedma
Reviewed By: eugenis, efriedma
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D67205
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371138 91177308-0d34-0410-b5e6-96231b3b80d8
Summary:
- Similar to the workaround in fix of PR30188, skip sinking common
lifetime markers of `alloca`. They are mostly left there after
inlining functions in branches.
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D66950
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370376 91177308-0d34-0410-b5e6-96231b3b80d8
This is the naive implementation of x86 BZHI/BEXTR instruction:
it takes input and bit count, and extracts low nbits up to bit width.
I.e. unlike shift it does not have any UB when nbits >= bitwidth.
Which means we don't need a while PHI here, simple select will do.
And if it's a select, it should then be trivial to fix codegen
to select it to BEXTR/BZHI.
See https://bugs.llvm.org/show_bug.cgi?id=34704
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370369 91177308-0d34-0410-b5e6-96231b3b80d8
Summary:
As it can be seen in the tests in D65143/D65144, even though we have formed an '@llvm.umul.with.overflow'
and got rid of potential for division-by-zero, the control flow remains, we still have that branch.
We have this condition:
```
// Don't fold i1 branches on PHIs which contain binary operators
// These can often be turned into switches and other things.
if (PN->getType()->isIntegerTy(1) &&
(isa<BinaryOperator>(PN->getIncomingValue(0)) ||
isa<BinaryOperator>(PN->getIncomingValue(1)) ||
isa<BinaryOperator>(IfCond)))
return false;
```
which was added back in rL121764 to help with `select` formation i think?
That check prevents us to flatten the CFG here, even though we know
we no longer need that guard and will be able to drop everything
but the '@llvm.umul.with.overflow' + `not`.
As it can be seen from tests, we end here because the `not` is being
sinked into the PHI's incoming values by InstCombine,
so we can't workaround this by hoisting it to after PHI.
Thus i suggest that we relax that check to not bailout if we'd get to hoist the `not`.
Reviewers: craig.topper, spatel, fhahn, nikic
Reviewed By: spatel
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D65147
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370349 91177308-0d34-0410-b5e6-96231b3b80d8
Summary:
DominatorTree is invalid after SimplifyCFG because of a missed `Changed = true` when simplifying a branch condition and removing an edge.
Resolves PR42272.
Reviewers: zhizhouy, manojgupta
Subscribers: jlebar, sanjoy.google, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D65490
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@367596 91177308-0d34-0410-b5e6-96231b3b80d8
Later code in TryToSimplifyUncondBranchFromEmptyBlock() assumes that
we have cleaned up unreachable blocks, but that was not happening
with this switch transform.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@367037 91177308-0d34-0410-b5e6-96231b3b80d8
This way it will be more obvious that the problem is both
in cost threshold and in hardcoded benefit check,
plus will show how the instsimplify cleans this all in the end.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366800 91177308-0d34-0410-b5e6-96231b3b80d8
This comes up in JPEG decoding, see e.g.
Figure F.12 – Extending the sign bit of a decoded value in V
of ITU T.81 (JPEG specification).
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366750 91177308-0d34-0410-b5e6-96231b3b80d8
Even if we formed @llvm.umul.with.overflow, we are still stuck
with that guard against div-by-zero, which is no longer needed,
because we didn't flatten the CFG.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366749 91177308-0d34-0410-b5e6-96231b3b80d8
If the blockaddress is not destoryed, the destination block will still
be marked as having its address taken, limiting further transformations.
I think there are other places where the dead blockaddress constants are kept
around, I'll look into that as follow up.
Reviewers: craig.topper, brzycki, davide
Reviewed By: brzycki, davide
Differential Revision: https://reviews.llvm.org/D64936
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366633 91177308-0d34-0410-b5e6-96231b3b80d8
Third time's the charm.
This was reverted in r363220 due to being suspected of an internal benchmark
regression and a test failure, none of which turned out to be caused by this.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363529 91177308-0d34-0410-b5e6-96231b3b80d8
SimplifyCFG has a bug that results in inconsistent prof branch_weights metadata
if unreachable switch cases are removed. This patch fixes this bug by making use
of the newly introduced SwitchInstProfUpdateWrapper class (see patch D62122).
A new test is created.
Differential Revision: https://reviews.llvm.org/D62186
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363527 91177308-0d34-0410-b5e6-96231b3b80d8
Also add baseline tests to show effect of later patches.
There were a couple of regressions here that were never caught,
but my patch set that this is a preparation to will fix them.
This is the third attempt to land this patch.
Differential Revision: https://reviews.llvm.org/D61150
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363319 91177308-0d34-0410-b5e6-96231b3b80d8
This reverts 363226 and 363227, both NFC intended
I swear I fixed the test case that is failing, and ran
the tests, but I will look into it again.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363229 91177308-0d34-0410-b5e6-96231b3b80d8
Also add baseline tests to show effect of later patches.
There were a couple of regressions here that were never caught,
but my patch set that this is a preparation to will fix them.
Differential Revision: https://reviews.llvm.org/D61150
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363226 91177308-0d34-0410-b5e6-96231b3b80d8
We have observed some failures with internal builds with this revision.
- Performance regressions:
- llvm's SingleSource/Misc evalloop shows performance regressions (although these may be red herrings).
- Benchmarks for Abseil's SwissTable.
- Correctness:
- Failures for particular libicu tests when building the Google AppEngine SDK (for PHP).
hwennborg has already been notified, and is aware of reproducer failures.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363220 91177308-0d34-0410-b5e6-96231b3b80d8
Even if one bit is defined, the code is not clear what it is suppose to do.
The test wants to assert that some bits are undef, but that's not what the IR does and I don't think it's even possible to do that in any meaningful way. It was added in D12497, so @reames might want to double check.
Differential Revision: https://reviews.llvm.org/D60859
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362499 91177308-0d34-0410-b5e6-96231b3b80d8
This was reverted in r360086 as it was supected of causing mysterious test
failures internally. However, it was never concluded that this patch was the
root cause.
> The code was previously checking that candidates for sinking had exactly
> one use or were a store instruction (which can't have uses). This meant
> we could sink call instructions only if they had a use.
>
> That limitation seemed a bit arbitrary, so this patch changes it to
> "instruction has zero or one use" which seems more natural and removes
> the need to special-case stores.
>
> Differential revision: https://reviews.llvm.org/D59936
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361811 91177308-0d34-0410-b5e6-96231b3b80d8
The old test was checking for a stupid subtract one that is a transform that
makes the code woorse.
The constant-islands-jump-table.ll test wants the code a specific way,
that makes sense, so I will submit code to fix that one.
Sorry that I really didn't know how to run the test suite before this.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361733 91177308-0d34-0410-b5e6-96231b3b80d8
Rather than gating on "isSwitchDense" (resulting in necessesarily
sparse lookup tables even when they were generated), always run
this quite cheap transform.
This transform is useful not just for generating tables.
LowerSwitch also wants this: read LowerSwitch.cpp:257.
Be careful to not generate worse code, by introducing a
SubThreshold heuristic.
Instead of just sorting by signed, generalize the finding of the
best base.
And now that it is run unconditionally, do not replicate its
functionality in SwitchToLookupTable (which could use a Sub
when having a hole is smaller, hence the SubThreshold
heuristic located in a single place).
This simplifies SwitchToLookupTable, and fixes
some ugly corner cases due to the use of signed numbers,
such as a table containing i16 32768 and 32769, of which
32769 would be interpreted as -32768, and now the code thinks
the table is size 65536.
(We still use unconditional subtraction when building a single-register mask,
but I think this whole block should go when the more general sparse
map is added, which doesn't leave empty holes in the table.)
And the reason test4 and test5 did not trigger was documented wrong:
it was because they were not considered sufficiently "dense".
Also, fix generation of invalid LLVM-IR: shl by bit-width.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361727 91177308-0d34-0410-b5e6-96231b3b80d8
This reverts r357452 (git commit 21eb771dcb5c11d7500fa6ad551c97a921997f05).
This was causing strange optimization-related test failures on an internal test. Will followup with more details offline.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360086 91177308-0d34-0410-b5e6-96231b3b80d8
As it's causing some bot failures (and per request from kbarton).
This reverts commit r358543/ab70da07286e618016e78247e4a24fcb84077fda.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358546 91177308-0d34-0410-b5e6-96231b3b80d8
The original commit caused false positives from AddressSanitizer's
use-after-scope checks, which have now been fixed in r358478.
> The code was previously checking that candidates for sinking had exactly
> one use or were a store instruction (which can't have uses). This meant
> we could sink call instructions only if they had a use.
>
> That limitation seemed a bit arbitrary, so this patch changes it to
> "instruction has zero or one use" which seems more natural and removes
> the need to special-case stores.
>
> Differential revision: https://reviews.llvm.org/D59936
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358483 91177308-0d34-0410-b5e6-96231b3b80d8
This revision causes tests to fail under ASAN. Since the cause of the failures
is not clear (could be ASAN, could be a Clang bug, could be a bug in this
revision), the safest course of action seems to be to revert while investigating.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@357667 91177308-0d34-0410-b5e6-96231b3b80d8
Summary:
When inserting an `unreachable` after a noreturn call, we must ensure
that it's not a musttail call to avoid breaking the IR invariants for
musttail calls.
Reviewers: fedor.sergeev, majnemer
Reviewed By: majnemer
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D60080
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@357485 91177308-0d34-0410-b5e6-96231b3b80d8
The code was previously checking that candidates for sinking had exactly
one use or were a store instruction (which can't have uses). This meant
we could sink call instructions only if they had a use.
That limitation seemed a bit arbitrary, so this patch changes it to
"instruction has zero or one use" which seems more natural and removes
the need to special-case stores.
Differential revision: https://reviews.llvm.org/D59936
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@357452 91177308-0d34-0410-b5e6-96231b3b80d8
These now verify that a given instruction has a specific source
location, rather than any old location. We want to make sure we
propagate the correct locations from one instruction to another.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@356217 91177308-0d34-0410-b5e6-96231b3b80d8
Fixes bug 38023: https://bugs.llvm.org/show_bug.cgi?id=38023
The SimplifyCFG pass will perform jump threading in some cases where
doing so is trivial and would simplify the CFG. When folding a series
of blocks with redundant conditional branches into an unconditional "critical
edge" block, it does not keep the debug location associated with the previous
conditional branch.
This patch fixes the bug described by copying the debug info from the
old conditional branch to the new unconditional branch instruction, and
adds a regression test for the SimplifyCFG pass that covers this case.
Patch by Stephen Tozer!
Differential Revision: https://reviews.llvm.org/D59206
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@355833 91177308-0d34-0410-b5e6-96231b3b80d8
Check that when SimplifyCFG is flattening a 'br', all their debug intrinsic instructions are removed, including any dbg.label referencing a label associated with the basic blocks being removed.
As the test case involves a CFG transformation, move it to the correct location.
Differential Revision: https://reviews.llvm.org/D57444
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353682 91177308-0d34-0410-b5e6-96231b3b80d8
The current llvm.mem.parallel_loop_access metadata has a problem in that
it uses LoopIDs. LoopID unfortunately is not loop identifier. It is
neither unique (there's even a regression test assigning the some LoopID
to multiple loops; can otherwise happen if passes such as LoopVersioning
make copies of entire loops) nor persistent (every time a property is
removed/added from a LoopID's MDNode, it will also receive a new LoopID;
this happens e.g. when calling Loop::setLoopAlreadyUnrolled()).
Since most loop transformation passes change the loop attributes (even
if it just to mark that a loop should not be processed again as
llvm.loop.isvectorized does, for the versioned and unversioned loop),
the parallel access information is lost for any subsequent pass.
This patch unlinks LoopIDs and parallel accesses.
llvm.mem.parallel_loop_access metadata on instruction is replaced by
llvm.access.group metadata. llvm.access.group points to a distinct
MDNode with no operands (avoiding the problem to ever need to add/remove
operands), called "access group". Alternatively, it can point to a list
of access groups. The LoopID then has an attribute
llvm.loop.parallel_accesses with all the access groups that are parallel
(no dependencies carries by this loop).
This intentionally avoid any kind of "ID". Loops that are clones/have
their attributes modifies retain the llvm.loop.parallel_accesses
attribute. Access instructions that a cloned point to the same access
group. It is not necessary for each access to have it's own "ID" MDNode,
but those memory access instructions with the same behavior can be
grouped together.
The behavior of llvm.mem.parallel_loop_access is not changed by this
patch, but should be considered deprecated.
Differential Revision: https://reviews.llvm.org/D52116
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@349725 91177308-0d34-0410-b5e6-96231b3b80d8
In PR39807 we incorrectly handle circumstances where calls are common'd
from conditional blocks into the parent BB. Calls that can be inlined
must always have DebugLocs, however we strip them during commoning, which
the IR verifier asserts on.
Fix this by using applyMergedLocation: it will perform the same DebugLoc
stripping of conditional Locs, but will also generate an unknown location
DebugLoc that satisfies the requirement for inlinable calls to always have
locations.
Some of the prior logic for selecting a DebugLoc is now likely redundant;
I'll generate a follow-up to remove it (involves editing more regression
tests).
Differential Revision: https://reviews.llvm.org/D54997
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@347782 91177308-0d34-0410-b5e6-96231b3b80d8
A call to @llvm.trap can be expected to be cold (i.e. unlikely to be
reached in a normal program execution).
Outlining paths which unconditionally trap is an important memory
saving. As the hot/cold splitting pass (imho) should not treat all
noreturn calls as cold, explicitly mark @llvm.trap cold so that it can
be outlined.
Split out of https://reviews.llvm.org/D54244.
Differential Revision: https://reviews.llvm.org/D54329
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@346885 91177308-0d34-0410-b5e6-96231b3b80d8
Roman Lebedev