[NFC][DebugInfo] Switch more call-sites to using iterator-insertion (#124283)

To finalise the "RemoveDIs" work removing debug intrinsics, we're
updating call sites that insert instructions to use i

[NFC][DebugInfo] Switch more call-sites to using iterator-insertion (#124283)

To finalise the "RemoveDIs" work removing debug intrinsics, we're
updating call sites that insert instructions to use iterators instead.
This set of changes are those where it's not immediately obvious that
just calling getIterator to fetch an iterator is correct, and one or two
places where more than one line needs to change.

Overall the same rule holds though: iterators generated for the start of
a block such as getFirstNonPHIIt need to be passed into insert/move
methods without being unwrapped/rewrapped, everything else can use
getIterator.

show more ...

[NFC][DebugInfo] Make some block-start-position methods return iterators (#124287)

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as

[NFC][DebugInfo] Make some block-start-position methods return iterators (#124287)

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as positions with iterators. A
number of these (such as getFirstNonPHIOrDbg) are sufficiently
infrequently used that we can just replace the pointer-returning version
with an iterator-returning version, hopefully without much/any
disruption.

Thus this patch has getFirstNonPHIOrDbg and
getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all
call-sites. There are no concerns about the iterators returned being
converted to Instruction*'s and losing the debug-info bit: because the
methods skip debug intrinsics, the iterator head bit is always false
anyway.

show more ...

[NFC][DebugInfo] Use iterator moveBefore at many call-sites (#123583)

As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNonPHI and sim

[NFC][DebugInfo] Use iterator moveBefore at many call-sites (#123583)

As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNonPHI and similar feed into
instruction insertion positions. Call-sites where that's necessary were
updated a year ago; but to ensure some type safety however, we'd like to
have all calls to moveBefore use iterators.

This patch adds a (guaranteed dereferenceable) iterator-taking
moveBefore, and changes a bunch of call-sites where it's obviously safe
to change to use it by just calling getIterator() on an instruction
pointer. A follow-up patch will contain less-obviously-safe changes.

We'll eventually deprecate and remove the instruction-pointer
insertBefore, but not before adding concise documentation of what
considerations are needed (very few).

show more ...

PatternMatch: migrate to CmpPredicate (#118534)

With the introduction of CmpPredicate in 51a895a (IR: introduce struct
with CmpInst::Predicate and samesign), PatternMatch is one of the first
key p

PatternMatch: migrate to CmpPredicate (#118534)

With the introduction of CmpPredicate in 51a895a (IR: introduce struct
with CmpInst::Predicate and samesign), PatternMatch is one of the first
key pieces of infrastructure that must be updated to match a CmpInst
respecting samesign information. Implement this change to Cmp-matchers.

This is a preparatory step in migrating the codebase over to
CmpPredicate. Since we no functional changes are desired at this stage,
we have chosen not to migrate CmpPredicate::operator==(CmpPredicate)
calls to use CmpPredicate::getMatching(), as that would have visible
impact on tests that are not yet written: instead, we call
CmpPredicate::operator==(Predicate), preserving the old behavior, while
also inserting a few FIXME comments for follow-ups.

show more ...

[SimpleLoopUnswitch] Fix LCSSA phi node invalidation

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

[LLVM] Add `Intrinsic::getDeclarationIfExists` (#112428)

Add `Intrinsic::getDeclarationIfExists` to lookup an existing
declaration of an intrinsic in a `Module`.

[LoopUnswitch] Remove redundant condition. (NFC) (#107893)

Remove redundant condition from '!A || (A && B)' to '!A || B'

Fixes: #99799

[DebugInfo][SimpleLoopUnswitch] Fix missing debug location updates for new terminators (#98789)

Fix #98787 .

[DebugInfo][SimpleLoopUnswitch] Fix missing debug location updates (#97662)

Fix #97559 .

For the change at line 1253, I propagate the debug location of the
terminator (i.e., the insertion point)

[DebugInfo][SimpleLoopUnswitch] Fix missing debug location updates (#97662)

Fix #97559 .

For the change at line 1253, I propagate the debug location of the
terminator (i.e., the insertion point) to the new phi. because `MergeBB`
is generated by splitting `ExitBB` several lines above, it only has the
terminator, which could provide a reasonable debug location.

For the change at line 2348, I switch the order of moving and cloning
`TI`. Because `NewTI` cloned from `TI` is inserted into the original
place where `TI` is, `NewTI` should preserve the origianl debug
location. At the same time, doing this allows us to propagate the debug
location to the new branch instruction replacing `NewTI` (the change at
line 2446).

show more ...

[IRBuilder] Don't include Module.h (NFC) (#97159)

This used to be necessary to fetch the DataLayout, but isn't anymore.

[RemoveDIs] Change remapDbgVariableRecord to remapDbgRecord (#91456)

We need to remap any DbgRecord, not just DbgVariableRecords.

This is the followup to #91447.

Co-authored-by: PietroGhg <pie

[RemoveDIs] Change remapDbgVariableRecord to remapDbgRecord (#91456)

We need to remap any DbgRecord, not just DbgVariableRecords.

This is the followup to #91447.

Co-authored-by: PietroGhg <pietro.ghiglio@codeplay.com>

show more ...

[Passes] Run SimpleLoopUnswitch after introducing invariant branches. (#81271)

IndVars may be able to replace a loop dependent condition with a loop
invariant one, but loop-unswitch runs before Ind

[Passes] Run SimpleLoopUnswitch after introducing invariant branches. (#81271)

IndVars may be able to replace a loop dependent condition with a loop
invariant one, but loop-unswitch runs before IndVars, so the invariant
check remains in the loop.

For an example, consider a read-only loop with a bounds check:
https://godbolt.org/z/8cdj4qhbG

This patch uses a approach similar to the way extra cleanup passes are
run on demand after vectorization (added in acea6e9cfa4c4a0e8678c7).

It introduces a new ShouldRunExtraSimpleLoopUnswitch analysis marker,
which IndVars can use to indicate that extra unswitching is beneficial.

ExtraSimpleLoopUnswitchPassManager uses this analysis to determine
whether to run its passes on a loop.

Compile-time impact (geomean) ranges from +0.0% to 0.02%
https://llvm-compile-time-tracker.com/compare.php?from=138c0beb109ffe47f75a0fe8c4dc2cdabe8a6532&to=19e6e99eeb280d426907ea73a21b139ba7225627&stat=instructions%3Au

Compile-time impact (geomean) of unconditionally running
SimpleLoopUnswitch ranges from +0.05% - +0.16%

https://llvm-compile-time-tracker.com/compare.php?from=138c0beb109ffe47f75a0fe8c4dc2cdabe8a6532&to=2930dfd5accdce2e6f8d5146ae4d626add2065a2&stat=instructions:u

Unconditionally running SimpleLoopUnswitch seems to indicate that there
are multiple other scenarios where we fail to run unswitching when
opportunities remain.


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

PR: https://github.com/llvm/llvm-project/pull/81271

show more ...

[RemoveDIs][NFC] Rename DPValue -> DbgVariableRecord (#85216)

This is the major rename patch that prior patches have built towards.
The DPValue class is being renamed to DbgVariableRecord, which re

[RemoveDIs][NFC] Rename DPValue -> DbgVariableRecord (#85216)

This is the major rename patch that prior patches have built towards.
The DPValue class is being renamed to DbgVariableRecord, which reflects
the updated terminology for the "final" implementation of the RemoveDI
feature. This is a pure string substitution + clang-format patch. The
only manual component of this patch was determining where to perform
these string substitutions: `DPValue` and `DPV` are almost exclusively
used for DbgRecords, *except* for:

- llvm/lib/target, where 'DP' is used to mean double-precision, and so
appears as part of .td files and in variable names. NB: There is a
single existing use of `DPValue` here that refers to debug info, which
I've manually updated.
- llvm/tools/gold, where 'LDPV' is used as a prefix for symbol
visibility enums.

Outside of these places, I've applied several basic string
substitutions, with the intent that they only affect DbgRecord-related
identifiers; I've checked them as I went through to verify this, with
reasonable confidence that there are no unintended changes that slipped
through the cracks. The substitutions applied are all case-sensitive,
and are applied in the order shown:

```
DPValue -> DbgVariableRecord
DPVal -> DbgVarRec
DPV -> DVR
```

Following the previous rename patches, it should be the case that there
are no instances of any of these strings that are meant to refer to the
general case of DbgRecords, or anything other than the DPValue class.
The idea behind this patch is therefore that pure string substitution is
correct in all cases as long as these assumptions hold.

show more ...

[RemoveDIs][NFC] Rename common interface functions for DPValues->DbgRecords (#84793)

As part of the effort to rename the DbgRecord classes, this patch
renames the widely-used functions that operate

[RemoveDIs][NFC] Rename common interface functions for DPValues->DbgRecords (#84793)

As part of the effort to rename the DbgRecord classes, this patch
renames the widely-used functions that operate on DbgRecords but refer
to DbgValues or DPValues in their names to refer to DbgRecords instead;
all such functions are defined in one of `BasicBlock.h`,
`Instruction.h`, and `DebugProgramInstruction.h`.

This patch explicitly does not change the names of any comments or
variables, except for where they use the exact name of one of the
renamed functions. The reason for this is reviewability; this patch can
be trivially examined to determine that the only changes are direct
string substitutions and any results from clang-format responding to the
changed line lengths. Future patches will cover renaming variables and
comments, and then renaming the classes themselves.

show more ...

[NFC][RemoveDIs] Insert instruction using iterators in Transforms/

As part of the RemoveDIs project we need LLVM to insert instructions using
iterators wherever possible, so that the iterators can c

[NFC][RemoveDIs] Insert instruction using iterators in Transforms/

As part of the RemoveDIs project we need LLVM to insert instructions using
iterators wherever possible, so that the iterators can carry a bit of
debug-info. This commit implements some of that by updating the contents of
llvm/lib/Transforms/Utils to always use iterator-versions of instruction
constructors.

There are two general flavours of update:
* Almost all call-sites just call getIterator on an instruction
* Several make use of an existing iterator (scenarios where the code is
actually significant for debug-info)
The underlying logic is that any call to getFirstInsertionPt or similar
APIs that identify the start of a block need to have that iterator passed
directly to the insertion function, without being converted to a bare
Instruction pointer along the way.

Noteworthy changes:
* FindInsertedValue now takes an optional iterator rather than an
instruction pointer, as we need to always insert with iterators,
* I've added a few iterator-taking versions of some value-tracking and
DomTree methods -- they just unwrap the iterator. These are purely
convenience methods to avoid extra syntax in some passes.
* A few calls to getNextNode become std::next instead (to keep in the
theme of using iterators for positions),
* SeparateConstOffsetFromGEP has it's insertion-position field changed.
Noteworthy because it's not a purely localised spelling change.

All this should be NFC.

show more ...

[SimpleLoopUnswitch] Remove callbacks (#73300)

After the removal of the legacyPM version of simple loop unswitch, there
is no longer a need for the callback mechanism to handle PM specific
tasks.

[SimpleLoopUnswitch] Remove callbacks (#73300)

After the removal of the legacyPM version of simple loop unswitch, there
is no longer a need for the callback mechanism to handle PM specific
tasks. This patch removes the callbacks to help simplify the code now
that they're no longer needed.

show more ...

[DebugInfo][RemoveDIs] Support cloning and remapping DPValues (#72546)

This patch adds support for CloneBasicBlock duplicating the DPValues
attached to instructions, and adds facilities to remap the

[DebugInfo][RemoveDIs] Support cloning and remapping DPValues (#72546)

This patch adds support for CloneBasicBlock duplicating the DPValues
attached to instructions, and adds facilities to remap them into their new
context. The plumbing to achieve this is fairly straightforwards and
mechanical.

I've also added illustrative uses to LoopUnrollRuntime, SimpleLoopUnswitch
and SimplifyCFG. The former only updates for the epilogue right now so I've
added CHECK lines just for the end of an unrolled loop (further updates
coming later). SimpleLoopUnswitch had no debug-info tests so I've added a
new one. The two modified parts of SimplifyCFG are covered by the two
modified SimplifyCFG tests.

These are scenarios where we have to do extra cloning for copying of
DPValues because they're no longer instructions, and remap them too.

show more ...

[NewPM] Remove SimpleLoopUnswitchLegacyPass (#72934)

This pass isn't used anywhere in upstream and thus doesn't have any test
coverage. For these reasons, remove it.

[SimpleLoopUnswitch] Fix exponential unswitch

When unswitching via invariant condition injection, we currently
mark the condition in the old loop, so that it does not get
unswitched again. However,

[SimpleLoopUnswitch] Fix exponential unswitch

When unswitching via invariant condition injection, we currently
mark the condition in the old loop, so that it does not get
unswitched again. However, if there are multiple branches for
which conditions can be injected, then we can do that for both
the old and new loop. This means that the number of unswitches
increases exponentially.

Change the handling to be more similar to partial unswitching,
where we instead mark the whole loop, rather than a single
condition. This means that we will only generate a linear number
of loops.

TBH I think even that is still highly undesirable, and we should
probably be unswitching all candidates at the same time, so that
we end up with only two loops. But at least this mitigates the
worst case.

The test case is a reduced variant that generates 1700 lines of IR
without this patch and 290 with it.

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

show more ...

[SimpleLoopUnswitch] Fix reversed branch during condition injection

The in-loop successor is only on the left after a potential condition
inversion. As we re-use the old condition as-is, we should a

[SimpleLoopUnswitch] Fix reversed branch during condition injection

The in-loop successor is only on the left after a potential condition
inversion. As we re-use the old condition as-is, we should also
reuse the old successors as-is.

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

show more ...

[NFC][RemoveDIs] Provide an iterator-taking split-block method

As per the stack of patches this is attached to, allow users of
BasicBlock::splitBasicBlock to provide an iterator for a position, inst

[NFC][RemoveDIs] Provide an iterator-taking split-block method

As per the stack of patches this is attached to, allow users of
BasicBlock::splitBasicBlock to provide an iterator for a position, instead
of just an instruction pointer. This is to fit with my proposal for how to
get rid of debug intrinsics [0]. There are other call-sites that would need
to change, but this is sufficient for a stage2clang self host and some
other C++ projects to build identical binaries, in the context of the whole
remove-DIs project.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939

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

show more ...

[NFC][RemoveDIs] Prefer iterator-insertion over instructions

Continuing the patch series to get rid of debug intrinsics [0], instruction
insertion needs to be done with iterators rather than instruc

[NFC][RemoveDIs] Prefer iterator-insertion over instructions

Continuing the patch series to get rid of debug intrinsics [0], instruction
insertion needs to be done with iterators rather than instruction pointers,
so that we can communicate information in the iterator class. This patch
adds an iterator-taking insertBefore method and converts various call sites
to take iterators. These are all sites where such debug-info needs to be
preserved so that a stage2 clang can be built identically; it's likely that
many more will need to be changed in the future.

At this stage, this is just changing the spelling of a few operations,
which will eventually become signifiant once the debug-info bearing
iterator is used.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939

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

show more ...

TTI: Pass function to hasBranchDivergence in a few passes

https://reviews.llvm.org/D152033

[SimpleLoopUnswitch] Unswitch AND/OR conditions of selects

If a select's condition is a AND/OR, we can unswitch invariant operands.
This patch uses existing logic from unswitching AND/OR's for branc

[SimpleLoopUnswitch] Unswitch AND/OR conditions of selects

If a select's condition is a AND/OR, we can unswitch invariant operands.
This patch uses existing logic from unswitching AND/OR's for branch
conditions.

This patch fixes the Cost computation for unswitching selects to have
the cost of the entire loop, since unswitching selects do not remove
branches. This is required for this patch because otherwise, there are
cases where unswitching selects of AND/OR is beating out unswitching of
branches.

This patch also prevents unswitching of logical AND/OR selects. This
should instead be done by unswitching of AND/OR branch conditions.

Reviewed By: nikic

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

show more ...

[SimpleLoopUnswitch] Verify LoopInfo in turnGuardIntoBranch under a flag

A follow-up for 64397d8. Only do verification if VerifyLoopInfo is
set.