[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 ...

[LoopRotate] Use `poison` instead of `undef` as placeholder in debug info [NFC] (#119135)

The `poison` values are used to substitute debug information of values
moved from the original header into

[LoopRotate] Use `poison` instead of `undef` as placeholder in debug info [NFC] (#119135)

The `poison` values are used to substitute debug information of values
moved from the original header into the preheader that are no longer
available in the former.

show more ...

Reapply "[llvm][IR] Extend BranchWeightMetadata to track provenance o… (#95281)

…f weights" #95136

Reverts #95060, and relands #86609, with the unintended code generation
changes addressed.

T

Reapply "[llvm][IR] Extend BranchWeightMetadata to track provenance o… (#95281)

…f weights" #95136

Reverts #95060, and relands #86609, with the unintended code generation
changes addressed.

This patch implements the changes to LLVM IR discussed in
https://discourse.llvm.org/t/rfc-update-branch-weights-metadata-to-allow-tracking-branch-weight-origins/75032

In this patch, we add an optional field to MD_prof meatdata nodes for
branch weights, which can be used to distinguish weights added from
llvm.expect* intrinsics from those added via other methods, e.g. from
profiles or inserted by the compiler.

One of the major motivations, is for use with MisExpect diagnostics,
which need to know if branch_weight metadata originates from an
llvm.expect intrinsic. Without that information, we end up checking
branch weights multiple times in the case if ThinLTO + SampleProfiling,
leading to some inaccuracy in how we report MisExpect related
diagnostics to users.

Since we change the format of MD_prof metadata in a fundamental way, we
need to update code handling branch weights in a number of places.

We also update the lang ref for branch weights to reflect the change.

show more ...

Revert "[llvm][IR] Extend BranchWeightMetadata to track provenance of weights" (#95060)

Reverts llvm/llvm-project#86609

This change causes compile-time regressions for stage2 builds
(https://llv

Revert "[llvm][IR] Extend BranchWeightMetadata to track provenance of weights" (#95060)

Reverts llvm/llvm-project#86609

This change causes compile-time regressions for stage2 builds
(https://llvm-compile-time-tracker.com/compare.php?from=3254f31a66263ea9647c9547f1531c3123444fcd&to=c5978f1eb5eeca8610b9dfce1fcbf1f473911cd8&stat=instructions:u).
It also introduced unintended changes to `.text` which should be
addressed before relanding.

show more ...

[llvm][IR] Extend BranchWeightMetadata to track provenance of weights (#86609)

This patch implements the changes to LLVM IR discussed in

https://discourse.llvm.org/t/rfc-update-branch-weights-met

[llvm][IR] Extend BranchWeightMetadata to track provenance of weights (#86609)

This patch implements the changes to LLVM IR discussed in

https://discourse.llvm.org/t/rfc-update-branch-weights-metadata-to-allow-tracking-branch-weight-origins/75032

In this patch, we add an optional field to MD_prof metadata nodes for
branch weights, which can be used to distinguish weights added from
`llvm.expect*` intrinsics from those added via other methods, e.g.
from profiles or inserted by the compiler.

One of the major motivations, is for use with MisExpect diagnostics,
which need to know if branch_weight metadata originates from an
llvm.expect intrinsic. Without that information, we end up checking
branch weights multiple times in the case if ThinLTO + SampleProfiling,
leading to some inaccuracy in how we report MisExpect related
diagnostics to users.

Since we change the format of MD_prof metadata in a fundamental way, we
need to update code handling branch weights in a number of places.

We also update the lang ref for branch weights to reflect the change.

show more ...

[LoopUnroll] Consider convergence control tokens when unrolling (#91715)

- There is no restriction on a loop with controlled convergent
operations when
the relevant tokens are defined and used w

[LoopUnroll] Consider convergence control tokens when unrolling (#91715)

- There is no restriction on a loop with controlled convergent
operations when
the relevant tokens are defined and used within the loop.

- When a token defined outside a loop is used inside (also called a loop
convergence heart), unrolling is allowed only in the absence of
remainder or
runtime checks.

- When a token defined inside a loop is used outside, such a loop is
said to be
"extended". This loop can only be unrolled by also duplicating the
extended part
lying outside the loop. Such unrolling is disabled for now.

- Clean up loop hearts: When unrolling a loop with a heart, duplicating
the
heart will introduce multiple static uses of a convergence control token
in a
cycle that does not contain its definition. This violates the static
rules for
tokens, and needs to be cleaned up into a single occurrence of the
intrinsic.

- Spell out the initializer for UnrollLoopOptions to improve
readability.


Original implementation [D85605] by Nicolai Haehnle
<nicolai.haehnle@amd.com>.

show more ...

[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 ...

[llvm][profdata][NFC] Support 64-bit weights in ProfDataUtils (#86607)

Since some places, like SimplifyCFG, work with 64-bit weights, we supply
an API in ProfDataUtils to extract the weights accord

[llvm][profdata][NFC] Support 64-bit weights in ProfDataUtils (#86607)

Since some places, like SimplifyCFG, work with 64-bit weights, we supply
an API in ProfDataUtils to extract the weights accordingly.

We change the API slightly to disambiguate the 64-bit version from the
32-bit version.

show more ...

[LoopRotate] Set loop back edge weight to not less than exit weight (#86496)

Branch weight from sample-based PGO may be not inaccurate due to
sampling. If the loop body must be executed, then origi

[LoopRotate] Set loop back edge weight to not less than exit weight (#86496)

Branch weight from sample-based PGO may be not inaccurate due to
sampling. If the loop body must be executed, then origin loop back
edge weight must be not less than exit weight.

show more ...

[RemoveDIs][NFC] Rename DPMarker->DbgMarker (#85931)

Another trivial rename patch, the last big one for now, which renamed
DPMarkers to DbgMarkers. This required the field `DbgMarker` in
`Instruct

[RemoveDIs][NFC] Rename DPMarker->DbgMarker (#85931)

Another trivial rename patch, the last big one for now, which renamed
DPMarkers to DbgMarkers. This required the field `DbgMarker` in
`Instruction` to be renamed to `DebugMarker` to avoid a clash, but
otherwise was a simple string substitution of `s/DPMarker/DbgMarker` and
a manual renaming of `DPM` to `DM` in the few places where that acronym
was used for debug markers.

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] Move DPValue::filter -> filterDbgVars (#85208)

This patch changes DPValue::filter to be a non-member method
filterDbgVars. There are two reasons for this: firstly, the name of
DPV

[RemoveDIs][NFC] Move DPValue::filter -> filterDbgVars (#85208)

This patch changes DPValue::filter to be a non-member method
filterDbgVars. There are two reasons for this: firstly, the name of
DPValue is about to change to DbgVariableRecord, which will result in
every `for` loop that uses DPValue::filter to require a line break. This
is a small thing, but it makes the rename patch more difficult to
review, and is just generally more awkward for what is a fairly common
loop. Secondly, the intent is to later break up the DPValue class into
subclasses, at which point it would be better to have a non-member
function that allows template arguments for the cases we want to filter
with greater specificity.

show more ...

[RemoveDI][NFC] Rename DPValue->DbgRecord in comments and varnames (#84939)

This patch continues the ongoing rename work, replacing DPValue with
DbgRecord in comments and the names of variables, bo

[RemoveDI][NFC] Rename DPValue->DbgRecord in comments and varnames (#84939)

This patch continues the ongoing rename work, replacing DPValue with
DbgRecord in comments and the names of variables, both members and
fn-local. This is the most labour-intensive part of the rename, as it is
where the most decisions have to be made about whether a given comment
or variable is referring to DPValues (equivalent to debug variable
intrinsics) or DbgRecords (a catch-all for all debug intrinsics); these
decisions are not individually difficult, but comprise a fairly large
amount of text to review.

This patch still largely performs basic string substitutions followed by
clang-format; there are almost* no places where, for example, a comment
has been expanded or modified to reflect the semantic difference between
DPValues and DbgRecords. I don't believe such a change is generally
necessary in LLVM, but it may be useful in the docs, and so I'll be
submitting docs changes as a separate patch.

*In a few places, `dbg.values` was replaced with `debug intrinsics`.

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 ...

[RemoveDIs] Reapply 3fda50d3915, insert instructions using iterators

I'd reverted this in 6c7805d5d1 after a bad stage. Original commit
messsage follows:

[NFC][RemoveDIs] Bulk update utilities to i

[RemoveDIs] Reapply 3fda50d3915, insert instructions using iterators

I'd reverted this in 6c7805d5d1 after a bad stage. Original commit
messsage follows:

[NFC][RemoveDIs] Bulk update utilities to insert with iterators

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.

I've also switched DemotePHIToStack to take an optional iterator: it needs
to take an iterator, and having a no-insert-location behaviour appears to
be important. The constructors for ICmpInst and FCmpInst have been updated
too. They're the only instructions that take block _references_ rather than
pointers for certain calls, and a future patch is going to make use of
default-null block insertion locations.

All of this should be NFC.

show more ...

Revert "[NFC][RemoveDIs] Bulk update utilities to insert with iterators"

This reverts commit 3fda50d3915b2163a54a37b602be7783a89dd808.

Apparently I've missed a hunk while staging this; will back ou

Revert "[NFC][RemoveDIs] Bulk update utilities to insert with iterators"

This reverts commit 3fda50d3915b2163a54a37b602be7783a89dd808.

Apparently I've missed a hunk while staging this; will back out for now.

Picked up here: https://lab.llvm.org/buildbot/#/builders/139/builds/60429/steps/6/logs/stdio

show more ...

[NFC][RemoveDIs] Bulk update utilities to insert with iterators

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

[NFC][RemoveDIs] Bulk update utilities to insert with iterators

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.

I've also switched DemotePHIToStack to take an optional iterator: it needs
to take an iterator, and having a no-insert-location behaviour appears to
be important. The constructors for ICmpInst and FCmpInst have been updated
too. They're the only instructions that take block _references_ rather than
pointers for certain calls, and a future patch is going to make use of
default-null block insertion locations.

All of this should be NFC.

show more ...

[LoopRotate][coroutines] Avoid hoisting addresses of thread-local variables outside loops in coroutines (#81937)

Because loops in coroutines may have a co_await statement that
reschedules the corou

[LoopRotate][coroutines] Avoid hoisting addresses of thread-local variables outside loops in coroutines (#81937)

Because loops in coroutines may have a co_await statement that
reschedules the coroutine to another thread, we cannot cache addresses
of thread-local variables obtained inside a loop by moving the
computation of thoes addresses outside a loop.

Since LLVM doesn't have a model for coroutine memory accesses, this
patch fixes this bug by disabling this optimization for coroutines in
the same way as https://reviews.llvm.org/D135550 and
https://reviews.llvm.org/D151774.

show more ...

[RemoveDIs][NFC] Introduce DbgRecord base class [1/3] (#78252)

Patch 1 of 3 to add llvm.dbg.label support to the RemoveDIs project. The
patch stack adds a new base class

-> 1. Add DbgRecord

[RemoveDIs][NFC] Introduce DbgRecord base class [1/3] (#78252)

Patch 1 of 3 to add llvm.dbg.label support to the RemoveDIs project. The
patch stack adds a new base class

-> 1. Add DbgRecord base class for DPValue and the not-yet-added
DPLabel class.
2. Add the DPLabel class.
3. Enable dbg.label conversion and add support to passes.

Patches 1 and 2 are NFC.

In the near future we also will rename DPValue to DbgVariableRecord and
DPLabel to DbgLabelRecord, at which point we'll overhaul the function
names too. The name DPLabel keeps things consistent for now.

show more ...

[DebugInfo][RemoveDIs] Don't allocate one DPMarker per instruction (#79345)

This is an optimisation patch that shouldn't have any functional effect.
There's no need for all instructions to have a DP

[DebugInfo][RemoveDIs] Don't allocate one DPMarker per instruction (#79345)

This is an optimisation patch that shouldn't have any functional effect.
There's no need for all instructions to have a DPMarker attached to them,
because not all instructions have adjacent DPValues (aka dbg.values).

This patch inserts the appropriate conditionals into functions like
BasicBlock::spliceDebugInfo to ensure we don't step on a null pointer when
there isn't a DPMarker allocated. Mostly, this is a case of calling
createMarker occasionally, which will create a marker on an instruction
if there isn't one there already.

Also folded into this is the use of adoptDbgValues, which is a natural
extension: if we have a sequence of instructions and debug records:

%foo = add i32 %0,...
# dbg_value { %foo, ...
# dbg_value { %bar, ...
%baz = add i32 %...
%qux = add i32 %...

and delete, for example, the %baz instruction, then the dbg_value records
would naturally be transferred onto the %qux instruction (they "fall down"
onto it). There's no point in creating and splicing DPMarkers in the case
shown when %qux doesn't have a DPMarker already, we can instead just change
the owner of %baz's DPMarker from %baz to %qux. This also avoids calling
setParent on every DPValue.

Update LoopRotationUtils: it was relying on each instruction having it's
own distinct end(), so that we could express ranges and lack-of-ranges.
That's no longer true though: so switch to storing the range of DPValues on
the next instruction when we want to consider it's range next time around
the loop (see the nearby comment).

show more ...

[DebugInfo][RemoveDIs] Switch some insertion routines to use iterators (#75330)

As part of RemoveDIs, we need instruction insertion to be done with
iterators rather than instruction pointers, so th

[DebugInfo][RemoveDIs] Switch some insertion routines to use iterators (#75330)

As part of RemoveDIs, we need instruction insertion to be done with
iterators rather than instruction pointers, so that we can communicate
some debug-info facts about the position. This patch is an entirely
mechanical replacement of Instruction * with BasicBlock::iterator, plus
using insertBefore to insert some instructions because we don't have
iterator-taking constructors yet.

Sadly it's not NFC because it causes dbg.value intrinsics / their
DPValue equivalents to shift location.

show more ...

[DebugInfo][RemoveDIs] Avoid crash and output-difference in loop-rotate (#74093)

Avoid editing a range of DPValues and then remapping them. This occurs
when we try to de-duplicate dbg.values, but t

[DebugInfo][RemoveDIs] Avoid crash and output-difference in loop-rotate (#74093)

Avoid editing a range of DPValues and then remapping them. This occurs
when we try to de-duplicate dbg.values, but then re-use the same
iterator range. We can instead remap them, and then erase any
duplicates.

At the same time refactor the computation of seen-intrinsic hashes, and
account for a peculiarity of loop-rotates existing behaviour: it will
only deduplicate dbg.values that are immediately before the preheaders
branch instruction, not just any dbg.value in the preheader.

show more ...

[DebugInfo][RemoveDIs] Instrument loop-rotate for DPValues (#72997)

Loop-rotate manually maintains dbg.value intrinsics -- it also needs to
manually maintain the replacement for dbg.value intrinsic

[DebugInfo][RemoveDIs] Instrument loop-rotate for DPValues (#72997)

Loop-rotate manually maintains dbg.value intrinsics -- it also needs to
manually maintain the replacement for dbg.value intrinsics, DPValue
objects. For the most part this patch adds parallel implementations
using the new type Some extra juggling is needed when loop-rotate hoists
loop-invariant instructions out of the loop: the DPValues attached to
such an instruction need to get rotated but not hoisted. Exercised by
the new test function invariant_hoist in dbgvalue.ll.

There's also a "don't insert duplicate debug intrinsics" facility in
LoopRotate. The value and correctness of this isn't clear, but to
continue preserving behaviour that's now tested in the "tak_dup"
function in dbgvalue.ll.

Other things in this patch include a helper DebugVariable constructor
for DPValues, a insertDebugValuesForPHIs handler for RemoveDIs
(exercised by the new tests), and beefing up the dbg.value checking in
dbgvalue.ll to ensure that each record is tested (and that there's an
implicit check-not).

show more ...

Add setBranchWeigths convenience function. NFC (#72446)

Add `setBranchWeights` convenience function to ProfDataUtils.h and use
it where appropriate.

LoopRotationUtils: Special case zero-branch weight cases (#66681)

The formula I added to LoopRotationUtils does not produce reasonable
results if some of the branch weights are zero. Add special ca

LoopRotationUtils: Special case zero-branch weight cases (#66681)

The formula I added to LoopRotationUtils does not produce reasonable
results if some of the branch weights are zero. Add special case
handling for this.

This fixes https://github.com/llvm/llvm-project/issues/66675

show more ...