Commit Graph

258 Commits

Author SHA1 Message Date
Thomas Lively c339250e12 [InstCombine] InstCombine and InstSimplify for minimum and maximum
Summary: Depends on D52765

Reviewers: aheejin, dschuff

Subscribers: llvm-commits

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

llvm-svn: 344799
2018-10-19 19:01:26 +00:00
George Burgess IV 40dc63e1f0 [Analysis] Make LocationSizes carry an 'imprecise' bit
There are places where we need to merge multiple LocationSizes of
different sizes into one, and get a sensible result.

There are other places where we want to optimize aggressively based on
the value of a LocationSizes (e.g. how can a store of four bytes be to
an area of storage that's only two bytes large?)

This patch makes LocationSize hold an 'imprecise' bit to note whether
the LocationSize can be treated as an upper-bound and lower-bound for
the size of a location, or just an upper-bound.

This concludes the series of patches leading up to this. The most recent
of which is r344108.

Fixes PR36228.

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

llvm-svn: 344114
2018-10-10 06:39:40 +00:00
Max Kazantsev b2724d9af8 [NFC] Add Requires: asserts where needed
llvm-svn: 342108
2018-09-13 04:43:24 +00:00
Max Kazantsev 0e0e19c980 [NFC] Use expensive asserts in relevant LICM tests
llvm-svn: 342107
2018-09-13 04:00:39 +00:00
Philip Reames cb8b3278e5 [AST] Generalize argument specific aliasing
AliasSetTracker has special case handling for memset, memcpy and memmove which pre-existed argmemonly on functions and readonly and writeonly on arguments. This patch generalizes it using the AA infrastructure to any call correctly annotated.

The motivation here is to cut down on confusion, not performance per se. For most instructions, there is a direct mapping to alias set. However, this is not guaranteed by the interface and was not in fact true for these three intrinsics *and only these three intrinsics*. I kept getting myself confused about this invariant, so I figured it would be good to clearly distinguish between a instructions and alias sets. Calls happened to be an easy target.

The nice side effect is that custom implementations of memset/memcpy/memmove - including wrappers discovered by IPO - can now be optimized the same as builts by LICM.

Note: The actual removal of the memset/memtransfer specific handling will happen in a follow on NFC patch.  It was originally part of this one, but separate for ease of review and rebase.

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

llvm-svn: 341713
2018-09-07 21:36:11 +00:00
Philip Reames 1887c40b22 Add a todo and tests to Address a review commnt from D50925 [NFC]
llvm-svn: 340978
2018-08-29 22:09:21 +00:00
Philip Reames f562fc8dbf [LICM] Hoist stores of invariant values to invariant addresses out of loops
Teach LICM to hoist stores out of loops when the store writes to a location otherwise unused in the loop, writes a value which is invariant, and is guaranteed to execute if the loop is entered.

Worth noting is that this transformation is partially overlapping with the existing promotion transformation. Reasons this is worthwhile anyway include:
 * For multi-exit loops, this doesn't require duplication of the store.
 * It kicks in for case where we can't prove we exit through a normal exit (i.e. we may throw), but can prove the store executes before that possible side exit.

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

llvm-svn: 340974
2018-08-29 21:49:30 +00:00
Hans Wennborg e0f3e9283f LoopSink: Don't sink into blocks without an insertion point (PR38462)
In the PR, LoopSink was trying to sink into a catchswitch block, which
doesn't have a valid insertion point.

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

llvm-svn: 340900
2018-08-29 06:55:27 +00:00
Philip Reames 9ec15faf20 [LICM] Hoist an invariant_start out of loops if there are no stores executed before it
Once the invariant_start is reached, we know that no instruction *after* it can modify the memory. So, if we can prove the location isn't read *between entry into the loop and the execution of the invariant_start*, we can execute the invariant_start before entering the loop.

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

llvm-svn: 340617
2018-08-24 16:24:48 +00:00
Philip Reames 6a2a5c99c7 [LICM] Fix a test so it actualy checks what was meant [NFC]
llvm-svn: 340344
2018-08-21 21:27:26 +00:00
Max Kazantsev 097ef69182 [LICM] Hoist guards with invariant conditions
This patch teaches LICM to hoist guards from the loop if they are guaranteed to execute and
if there are no side effects that could prevent that.

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

llvm-svn: 340256
2018-08-21 08:11:31 +00:00
Max Kazantsev f1dc867396 [NFC] Add some LICM tests
llvm-svn: 340254
2018-08-21 07:37:02 +00:00
Philip Reames a5a8546ac6 [AST] Mark invariant.starts as being readonly
These intrinsics are modelled as writing for control flow purposes, but they don't actually write to any location. Marking these - as we did for guards - allows LICM to hoist loads out of loops containing invariant.starts.

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

llvm-svn: 340245
2018-08-21 00:55:35 +00:00
Philip Reames 578c64da0c [LICM] Add tests from D50786 [NFC]
Exercise more use of volatiles to illustrate that nothing changes as we tweak how we detect them.

llvm-svn: 340244
2018-08-21 00:42:07 +00:00
Philip Reames efdd0a426a [LICM][NFC] Add tests from D50730
Landing tests so corresponding change can show effects clearly.  see
D50730 [AST] Generalize argument specific aliasing

llvm-svn: 340243
2018-08-21 00:37:09 +00:00
Philip Reames 4009487e5c [LICM] More tests for D50925 [NFC]
This time, the corresponding cases where we can hoist (store-like) calls out of loops.

llvm-svn: 340242
2018-08-21 00:14:14 +00:00
Philip Reames 529a590bce [LICM][Tests] Add tests for store hoisting [NFC]
https://reviews.llvm.org/D50925 will be rebased on top of this.

llvm-svn: 340233
2018-08-20 23:37:59 +00:00
Anna Thomas 1962621a7e [LICM] Add a diagnostic analysis for identifying alias information
Summary:
Currently, in LICM, we use the alias set tracker to identify if the
instruction (we're interested in hoisting) aliases with instruction that
modifies that memory location.

This patch adds an LICM alias analysis diagnostic tool that checks the
mod ref info of the instruction we are interested in hoisting/sinking,
with every instruction in the loop.  Because of O(N^2) complexity this
is now only a diagnostic tool to show the limitation we have with the
alias set tracker and is OFF by default.

Test cases show the difference with the diagnostic analysis tool, where
we're able to hoist out loads and readonly + argmemonly calls from the
loop, where the alias set tracker analysis is not able to hoist these
instructions out.

Reviewers: reames, mkazantsev, fedor.sergeev, hfinkel

Subscribers: llvm-commits

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

llvm-svn: 340026
2018-08-17 13:44:00 +00:00
Max Kazantsev 7b78d3920c [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

llvm-svn: 339984
2018-08-17 06:19:17 +00:00
Max Kazantsev cfa3e66b8e [NFC] Add tests to ensure that improvement of MustThrow analysis will not lead to problems in future
llvm-svn: 339983
2018-08-17 05:20:25 +00:00
Philip Reames 684fa57ef7 [MemLoc] Fix a bug causing any use of invariant.end to crash in LICM
The fix is fairly simple, but is says something unpleasant about the usage and testing of invariant.start/end scopes that this went undetected.  To put this in perspective, *any* invariant.end in a loop flowing through LICM crashed.  I haven't bothered to figure out just how far back this goes, but it's not caused by any of the recent changes.  We're probably talking months if not years.  

llvm-svn: 339936
2018-08-16 20:48:55 +00:00
Max Kazantsev 5a10d127b9 [AliasSetTracker] Do not treat experimental_guard intrinsic as memory writing instruction
The `experimental_guard` intrinsic has memory write semantics to model the thread-exiting
logic, but does not do any actual writes to memory. Currently, `AliasSetTracker` treats it as a
normal memory write. As result, a loop-invariant load cannot be hoisted out of loop because
the guard may possibly alias with it.

This patch makes `AliasSetTracker` so that it doesn't treat guards as memory writes.

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

llvm-svn: 339753
2018-08-15 06:21:02 +00:00
Philip Reames 85afd1a9a0 [LICM] Hoist assumes out of loops
If we have an assume which is known to execute and whose operand is invariant, we can lift that into the pre-header. So long as we don't change which paths the assume executes on, this is a legal transformation. It's likely to be a useful canonicalization as other transforms only look for dominating assumes.

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

llvm-svn: 339481
2018-08-10 22:21:56 +00:00
Max Kazantsev 4e9def57c7 [NFC] Add tests that demonstrate that MustExecute is fundamentally broken
llvm-svn: 339417
2018-08-10 09:20:46 +00:00
Philip Reames ca256d93fb [LICM] hoist fences out of loops w/o memory operations
The motivating case is an otherwise dead loop with a fence in it. At the moment, this goes all the way through the optimizer and we end up emitting an entirely pointless loop on x86. This case may seem a bit contrived, but we've seen it in real code as the result of otherwise reasonable lowering strategies combined w/thread local memory optimizations (such as escape analysis).

To handle this simple case, we can teach LICM to hoist must execute fences when there is no other memory operation within the loop.

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

llvm-svn: 339378
2018-08-09 20:18:42 +00:00
Philip Reames 954eab1087 [LICM] Add tests for future hoisting of fence instructions [NFC]
The main interesting case is a fence in an otherwise dead loop or one containing only arithmetic.  This can happen as a result of DSE or other transforms from seemingly reasonable initial IR.  

llvm-svn: 339310
2018-08-09 04:21:02 +00:00
Max Kazantsev c9dca6df78 [NFC] Add some tests on mustexec
llvm-svn: 339219
2018-08-08 04:40:47 +00:00
Philip Reames c792e197b4 [LICM] Strengthen assume hoisting tests [NFC]
As requested in review of https://reviews.llvm.org/D50364

llvm-svn: 339159
2018-08-07 17:54:36 +00:00
Philip Reames 94b29601ef [LICM] Further strengthen tests for hoisting guards and invariant.starts [NFC]
llvm-svn: 339062
2018-08-06 21:39:43 +00:00
Philip Reames 9d7bb2f700 [LICM] Strengthen invariant.start hoisting tests [NFC]
llvm-svn: 339057
2018-08-06 21:18:34 +00:00
Philip Reames 81c7dc93d2 [LICM] Add tests highlighting missing hoists for intrinsics [NFC]
llvm-svn: 339054
2018-08-06 21:06:15 +00:00
Max Kazantsev 2dbbd64cb7 Re-enable "[ValueTracking] Teach isKnownNonNullFromDominatingCondition about AND"
The patch was reverted because of bug detected by sanitizer. The bug is fixed,
respective tests added.

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

llvm-svn: 339005
2018-08-06 11:14:18 +00:00
Max Kazantsev 3271f379a9 Revert rL338990 to see if it causes sanitizer failures
Multiple failues reported by sanitizer-x86_64-linux, seem to be caused by this
patch. Reverting to see if they sustain without it.

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

llvm-svn: 338994
2018-08-06 08:10:28 +00:00
Max Kazantsev 34b0666be9 [ValueTracking] Teach isKnownNonNullFromDominatingCondition about AND
`isKnownNonNullFromDominatingCondition` is able to prove non-null basing on `br` or `guard`
by `%p != null` condition, but is unable to do so basing on `(%p != null) && %other_cond`.
This patch allows it to do so.

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

llvm-svn: 338990
2018-08-06 06:11:36 +00:00
Philip Reames 5937368d4f [LICM] Remove unneccessary safety check to increase sinking effectiveness
This one requires a bit of explaination.  It's not every day you simply delete code to implement an optimization.  :)

The transform in question is sinking an instruction from a loop to the uses in loop exiting blocks.  We know (from LCSSA) that all of the uses outside the loop must be phi nodes, and after predecessor splitting, we know all phi users must have a single operand.  Since the use must be strictly dominated by the def, we know from the definition of dominance/ssa that the exit block must execute along a (non-strict) subset of paths which reach the def.  As a result, duplicating a potentially faulting instruction can not *introduce* a fault that didn't previously exist in the program.  

The full story is that this patch builds on "rL338671: [LICM] Factor out fault legality from canHoistOrSinkInst [NFC]" which pulled this logic out of a common helper routine.  As best I can tell, this check was originally added to the helper function for hoisting legality, later an incorrect fastpath for loads/calls was added, and then the bug was fixed by duplicating the fault safety check in the hoist path.  This left the redundant check in the common code to pessimize sinking for no reason.  I split it out in an NFC, and am not removing the unneccessary check.  I wanted there to be something easy to revert in case I missed something.

Reviewed by: Anna Thomas (in person)

llvm-svn: 338794
2018-08-03 00:21:56 +00:00
Philip Reames 24b13cb06d [LICM] Expand tests to highlight an oddity in sinking implementation
llvm-svn: 338670
2018-08-02 03:54:29 +00:00
Gabor Buella da4a966e1c NFC - Various typo fixes in tests
llvm-svn: 336268
2018-07-04 13:28:39 +00:00
Bjorn Pettersson 8dd6cf711f [DebugInfo] Corrections for salvageDebugInfo
Summary:
When salvaging a dbg.declare/dbg.addr we should not add
DW_OP_stack_value to the DIExpression
(see test/Transforms/InstCombine/salvage-dbg-declare.ll).

Consider this example
  %vla = alloca i32, i64 2
  call void @llvm.dbg.declare(metadata i32* %vla, metadata !1, metadata !DIExpression())

Instcombine will turn it into
  %vla1 = alloca [2 x i32]
  %vla1.sub = getelementptr inbounds [2 x i32], [2 x i32]* %vla, i64 0, i64 0
  call void @llvm.dbg.declare(metadata [2 x i32]* %vla1.sub, metadata !19, metadata !DIExpression())

If the GEP can be eliminated, then the dbg.declare will be salvaged
and we should get
  %vla1 = alloca [2 x i32]
  call void @llvm.dbg.declare(metadata [2 x i32]* %vla1, metadata !19, metadata !DIExpression())

The problem was that salvageDebugInfo did not recognize dbg.declare
as being indirect (%vla1 points to the value, it does not hold the
value), so we incorrectly got
  call void @llvm.dbg.declare(metadata [2 x i32]* %vla1, metadata !19, metadata !DIExpression(DW_OP_stack_value))

I also made sure that llvm::salvageDebugInfo and
DIExpression::prependOpcodes do not add DW_OP_stack_value to
the DIExpression in case no new operands are added to the
DIExpression. That way we avoid to, unneccessarily, turn a
register location expression into an implicit location expression
in some situations (see test11 in test/Transforms/LICM/sinking.ll).

Reviewers: aprantl, vsk

Reviewed By: aprantl, vsk

Subscribers: JDevlieghere, llvm-commits

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

llvm-svn: 336191
2018-07-03 11:29:00 +00:00
Stanislav Mekhanoshin d8c9374797 Fix invariant fdiv hoisting in LICM
FDiv is replaced with multiplication by reciprocal and invariant
reciprocal is hoisted out of the loop, while multiplication remains
even if invariant.

Switch checks for all invariant operands and only invariant
denominator to fix the issue.

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

llvm-svn: 335411
2018-06-23 04:01:28 +00:00
David Stenberg 05b6a53340 [MustExecute] Fix a debug invariant issue in isGuaranteedToExecute()
Summary:
Look past debug intrinsics when querying whether an instruction is the
first instruction in the header block. The commit includes a reproducer
for a case where LICM would not hoist an instruction, due to the presence
of the intrinsic.

A caveat with this commit is that the check will not work properly if
the instruction at hand is a debug intrinsic. I assume that no one
depends on isGuaranteedToExecute() to return true for debug intrinsics
for these cases (and that this might be an indication of another debug
invariant issue), so I thought that it was not worth adding that extra
bit of complexity.

Reviewers: reames, anna

Reviewed By: anna

Subscribers: llvm-commits

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

llvm-svn: 333274
2018-05-25 13:02:59 +00:00
Jun Bum Lim dfbe6fa832 [LICM] Preserve DT and LoopInfo specifically
Summary:
In LICM, CFG could be changed in splitPredecessorsOfLoopExit(), which update
only DT and LoopInfo. Therefore, we should preserve only DT and LoopInfo specifically,
instead of all analyses that depend on the CFG (setPreservesCFG()).

This change should fix PR37323.

Reviewers: uabelho, davide, dberlin, Ka-Ka

Reviewed By: dberlin

Subscribers: mzolotukhin, bjope, mcrosier, llvm-commits

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

llvm-svn: 333198
2018-05-24 15:58:34 +00:00
Serguei Katkov 5095883fe9 [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

llvm-svn: 332695
2018-05-18 04:56:28 +00:00
Shiva Chen 2c864551df [DebugInfo] Add DILabel metadata and intrinsic llvm.dbg.label.
In order to set breakpoints on labels and list source code around
labels, we need collect debug information for labels, i.e., label
name, the function label belong, line number in the file, and the
address label located. In order to keep these information in LLVM
IR and to allow backend to generate debug information correctly.
We create a new kind of metadata for labels, DILabel. The format
of DILabel is

!DILabel(scope: !1, name: "foo", file: !2, line: 3)

We hope to keep debug information as much as possible even the
code is optimized. So, we create a new kind of intrinsic for label
metadata to avoid the metadata is eliminated with basic block.
The intrinsic will keep existing if we keep it from optimized out.
The format of the intrinsic is

llvm.dbg.label(metadata !1)

It has only one argument, that is the DILabel metadata. The
intrinsic will follow the label immediately. Backend could get the
label metadata through the intrinsic's parameter.

We also create DIBuilder API for labels to be used by Frontend.
Frontend could use createLabel() to allocate DILabel objects, and use
insertLabel() to insert llvm.dbg.label intrinsic in LLVM IR.

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

Patch by Hsiangkai Wang.

llvm-svn: 331841
2018-05-09 02:40:45 +00:00
Philip Reames 5b39acd111 [LICM] Compute a must execute property for the prefix of the header as we go
Computing this property within the existing walk ensures that the cost is linear with the size of the block. If we did this from within isGuaranteedToExecute, it would be quadratic without some very fancy caching.

This allows us to reliably catch a hoistable instruction within a header which may throw at some point *after* our hoistable instruction. It doesn't do anything for non-header cases, but given how common single block loops are, this seems very worthwhile.

llvm-svn: 331557
2018-05-04 21:35:00 +00:00
Philip Reames e4ec473b3f [MustExecute/LICM] Special case first instruction in throwing header
We currently have a hard to solve analysis problem around the order of instructions within a potentially throwing block.  We can't cheaply determine whether a given instruction is before the first potential throw in the block.  While we're working on that in the background, special case the first instruction within the header.

why this particular special case?  Well, headers are guaranteed to execute if the loop does, and it turns out we tend to produce this form in practice.

In a follow on patch, I tend to extend LICM with an alternate approach which works for any instruction in the header before the first throw, but this is the best I can come up with other users of the analysis (such as store promotion.)

Note: I can't show the difference in the analysis result since we're ORing in the expensive instruction walk used by SCEV.  Using the full walk is not suitable for a general solution.
llvm-svn: 331079
2018-04-27 20:44:01 +00:00
Anastasis Grammenos 3a589103a4 [LICM] Salvage DI from dying Instructions
LICM deletes trivially dead instructions which it won't attempt to sink.
Attempt to salvage debug values which reference these instructions.

llvm-svn: 327800
2018-03-18 15:59:19 +00:00
Philip Reames 8a106272e8 [LICM/mustexec] Extend first iteration must execute logic to fcmps
This builds on the work from https://reviews.llvm.org/D44287. It turned out supporting fcmp was much easier than I realized, so let's do that now.

As an aside, our -O3 handling of a floating point IVs leaves a lot to be desired. We do convert the float IV to an integer IV, but do so late enough that many other optimizations are missed (e.g. we don't vectorize).

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

llvm-svn: 327722
2018-03-16 16:33:49 +00:00
Philip Reames a21d5f1e18 [LICM] Ignore exits provably not taken on first iteration when computing must execute
It is common to have conditional exits within a loop which are known not to be taken on some iterations, but not necessarily all. This patches extends our reasoning around guaranteed to execute (used when establishing whether it's safe to dereference a location from the preheader) to handle the case where an exit is known not to be taken on the first iteration and the instruction of interest *is* known to be taken on the first iteration.

This case comes up in two major ways:
* If we have a range check which we've been unable to eliminate, we frequently know that it doesn't fail on the first iteration.
* Pass ordering. We may have a check which will be eliminated through some sequence of other passes, but depending on the exact pass sequence we might never actually do so or we might miss other optimizations from passes run before the check is finally eliminated.

The initial version (here) is implemented via InstSimplify. At the moment, it catches a few cases, but misses a lot too. I added test cases for missing cases in InstSimplify which I'll follow up on separately. Longer term, we should probably wire SCEV through to here to get much smarter loop aware simplification of the first iteration predicate.

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

llvm-svn: 327664
2018-03-15 21:04:28 +00:00
Jun Bum Lim 144eb593dd [LICM] update BlockColors after splitting predecessors
Update BlockColors after splitting predecessors. Do not allow splitting
EHPad for sinking when the BlockColors is not empty, so we can
simply assign predecessor's color to the new block.

Fixes PR36184

llvm-svn: 324916
2018-02-12 17:56:55 +00:00
Michael Zolotukhin cae66ba5f8 The xfailed test from r324448 passed on one of the bots: remove it entirely for now.
llvm-svn: 324451
2018-02-07 06:54:11 +00:00