163 Commits

Author SHA1 Message Date
Orlando Cazalet-Hyams
435d4c12de Reapply [RemoveDIs] Read/write DbgRecords directly from/to bitcode (#83251)
Reaplying after revert in #85382 (861ebe6446296c96578807363aa292c69d827773).
Fixed intermittent test failure by avoiding piping output in some RUN lines.

If --write-experimental-debuginfo-iterators-to-bitcode is true (default false)
and --expermental-debuginfo-iterators is also true then the new debug info
format (non-instruction records) is written to bitcode directly.

Added the following records:

    FUNC_CODE_DEBUG_RECORD_LABEL
    FUNC_CODE_DEBUG_RECORD_VALUE
    FUNC_CODE_DEBUG_RECORD_DECLARE
    FUNC_CODE_DEBUG_RECORD_ASSIGN
    FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE

The last one has an abbrev in FUNCTION_BLOCK BLOCK_INFO. Incidentally, this uses
the last value available without widening the code-length for FUNCTION_BLOCK
from 4 to 5 bits.

Records are formatted as follows:

    All DbgRecord start with:
      1. DILocation

      FUNC_CODE_DEBUG_RECORD_LABEL
        2. DILabel

      DPValues then share common fields:
        2. DILocalVariable
        3. DIExpression

        FUNC_CODE_DEBUG_RECORD_VALUE
          4. Location Metadata

        FUNC_CODE_DEBUG_RECORD_DECLARE
          4. Location Metadata

        FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE
	  4. Location Value (single)

        FUNC_CODE_DEBUG_RECORD_ASSIGN
	  4. Location Metadata
	  5. DIAssignID
	  6. DIExpression (address)
	  7. Location Metadata (address)

Encoding the DILocation metadata reference directly appeared to yield smaller
bitcode files than encoding the operands seperately (as is done with instruction
DILocations).

FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE is by far the most common DbgRecord record
in optimized code (order of 5x-10x over other kinds). Unoptimized code should
only contain FUNC_CODE_DEBUG_RECORD_DECLARE.
2024-03-15 12:33:55 +00:00
Orlando Cazalet-Hyams
861ebe6446
Revert "[RemoveDIs] Read/write DbgRecords directly from/to bitcode" (#85382)
Reverts llvm/llvm-project#83251

Buildbot: https://lab.llvm.org/buildbot/#/builders/139/builds/61485
2024-03-15 11:04:21 +00:00
Orlando Cazalet-Hyams
d6d3d96b65
[RemoveDIs] Read/write DbgRecords directly from/to bitcode (#83251)
If --write-experimental-debuginfo-iterators-to-bitcode is true (default false)
and --expermental-debuginfo-iterators is also true then the new debug info
format (non-instruction records) is written to bitcode directly.

Added the following records:

    FUNC_CODE_DEBUG_RECORD_LABEL
    FUNC_CODE_DEBUG_RECORD_VALUE
    FUNC_CODE_DEBUG_RECORD_DECLARE
    FUNC_CODE_DEBUG_RECORD_ASSIGN
    FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE

The last one has an abbrev in FUNCTION_BLOCK BLOCK_INFO. Incidentally, this uses
the last value available without widening the code-length for FUNCTION_BLOCK
from 4 to 5 bits.

Records are formatted as follows:

    All DbgRecord start with:
      1. DILocation

      FUNC_CODE_DEBUG_RECORD_LABEL
        2. DILabel

      DPValues then share common fields:
        2. DILocalVariable
        3. DIExpression

        FUNC_CODE_DEBUG_RECORD_VALUE
          4. Location Metadata

        FUNC_CODE_DEBUG_RECORD_DECLARE
          4. Location Metadata

        FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE
	  4. Location Value (single)

        FUNC_CODE_DEBUG_RECORD_ASSIGN
	  4. Location Metadata
	  5. DIAssignID
	  6. DIExpression (address)
	  7. Location Metadata (address)

Encoding the DILocation metadata reference directly appeared to yield smaller
bitcode files than encoding the operands seperately (as is done with instruction
DILocations).

FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE is by far the most common DbgRecord record
in optimized code (order of 5x-10x over other kinds). Unoptimized code should
only contain FUNC_CODE_DEBUG_RECORD_DECLARE.
2024-03-15 10:47:48 +00:00
Stephen Tozer
360da83858
[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`.
2024-03-13 16:39:35 +00:00
Stephen Tozer
15f3f446c5
[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.
2024-03-12 14:53:13 +00:00
Orlando Cazalet-Hyams
9997e03971
[RemoveDIs] Update DIBuilder to conditionally insert DbgRecords (#84739)
Have DIBuilder conditionally insert either debug intrinsics or DbgRecord
depending on the module's IsNewDbgInfoFormat flag. The insertion methods
now return a `DbgInstPtr` (a `PointerUnion<Instruction *, DbgRecord
*>`).

Add a unittest for both modes (I couldn't find an existing test testing
insertion behaviours specifically).

This patch changes the existing assumption that DbgRecords are only ever
inserted if there's an instruction to insert-before because clang
currently inserts debug intrinsics while CodeGening (like any other
instruction) meaning it'll try inserting to the end of a block without a
terminator. We already have machinery in place to maintain the
DbgRecords when a terminator is removed - these become "trailing
DbgRecords" which are re-attached when a new instruction is inserted.
All I've done is allow this state to occur while inserting DbgRecords
too, i.e., it's not only removing terminators that causes this valid
transient state, but inserting DbgRecords into incomplete blocks too.

The C API will be updated in follow up patches.

---

Note: this doesn't mean clang is emitting DbgRecords yet, because the
modules it creates are still always in the old debug mode. That will
come in a future patch.
2024-03-12 10:25:58 +00:00
Daniel Sanders
75790dd2d0
[RemoveDIs] Fix nullptr dereference in getFirstNonPHIIt() (#84595)
getFirstNonPHI() returns nullptr for blocks that lack a non-phi
(including a terminator) but getFirstNonPHIIt() may dereference its
result unconditionally. Return end() instead.

This came up for us downstream while correcting our getFirstNonPHI()
calls that intended to return the position after the phi's but before
the debug info to getFirstNonPHIIt(). The pass in question is populating
new BB's and hasn't added terminators yet.
2024-03-11 16:05:29 -07:00
Stephen Tozer
ed35ad18ae
[RemoveDIs][DebugInfo] Add DPValue checks to the verifier, prepare DPValue for parsing support (#79810)
As part of the RemoveDIs project, this patch adds support for checking
DPValues in the verifier. Although this is not strictly parsing-related,
and we currently automatically convert back to the old debug info format
immediately after parsing, we are approaching the point where the we can
operate end-to-end in the new debug info format, at which point it is
appropriate that we can actually validate modules in the new format.
This patch also contains some changes that aren't strictly
parsing-related, but are necessary class refactors for parsing support,
and are used in the verifier checks (i.e. changing the DILocalVariable
field to be a tracking MD reference, and adding a Verifier check to
confirm that it is a DILocalVariable).
2024-02-27 16:30:31 +00:00
Stephen Tozer
2e39b57837 Reapply "[RemoveDIs] Print non-intrinsic debug info in textual IR output (#79281)"
This reapplication changes debug intrinsic declaration removal to only take
place when printing final IR, so that the processing format of the Module
does not affect the output.

This reverts commit d128448efdd4e2bf3c9bc9a5b43ae642aa78026f.
2024-02-27 14:23:52 +00:00
Stephen Tozer
d128448efd Revert "Reapply "[RemoveDIs] Print non-intrinsic debug info in textual IR output (#79281)""
Reverted due to some test failures on some buildbots.

https://lab.llvm.org/buildbot/#/builders/67/builds/14669

This reverts commit aa436493ab7ad4cf323b0189c15c59ac9dc293c7.
2024-02-27 10:17:24 +00:00
Stephen Tozer
aa436493ab Reapply "[RemoveDIs] Print non-intrinsic debug info in textual IR output (#79281)"
Fixes the prior issue in which the symbol for a cl-arg was unavailable to
some binaries.

This reverts commit dc06d75ab27b4dcae2940fc386fadd06f70faffe.
2024-02-27 09:59:08 +00:00
Stephen Tozer
dc06d75ab2 Revert "[RemoveDIs] Print non-intrinsic debug info in textual IR output (#79281)"
Reverted due to failures on buildbots, where a new cl flag was placed
in the wrong file, resulting in link errors.

https://lab.llvm.org/buildbot/#/builders/198/builds/8548

This reverts commit 0b398256b3f72204ad1f7c625efe4990204e898a.
2024-02-26 18:49:18 +00:00
Stephen Tozer
0b398256b3
[RemoveDIs] Print non-intrinsic debug info in textual IR output (#79281)
This patch adds support for printing the proposed non-instruction debug
info ("RemoveDIs") out to textual IR. This patch does not add any
bitcode support, parsing support, or documentation.

Printing of the new format is controlled by a flag added in this patch,
`--write-experimental-debuginfo`, which defaults to false. The new
format will be printed *iff* this flag is true, so whether we use the IR
format is completely independent of whether we use non-instruction debug
info during LLVM passes (which is controlled by the
`--try-experimental-debuginfo-iterators` flag).

Even with the flag disabled, some existing tests need to be updated, as this
patch causes debug intrinsic declarations to be changed in a round trip,
such that they always appear at the end of a module and have no attributes
(this has no functional change on the module).

The design of this new IR format was proposed previously on
Discourse, and any further discussion about the design can still be
contributed there:

https://discourse.llvm.org/t/rfc-debuginfo-proposed-changes-to-the-textual-ir-representation-for-debug-values/73491
2024-02-26 18:22:05 +00:00
Orlando Cazalet-Hyams
3356818eed Reapply [RemoveDIs] Enable DPLabels conversion [3b/3] (#82639)
Enables conversion between llvm.dbg.label and DPLabel.
2024-02-26 09:09:52 +00:00
Jorge Gorbe Moya
c862e61206 Revert "[RemoveDIs] Enable DPLabels conversion [3b/3] (#82639)"
This reverts commit 71d47a0b00e9f48dc740556d7f452ffadf308731 because
it causes clang to crash in some cases. See repro posted at
71d47a0b00
2024-02-23 16:43:07 -08:00
Orlando Cazalet-Hyams
71d47a0b00
[RemoveDIs] Enable DPLabels conversion [3b/3] (#82639)
Enables conversion between llvm.dbg.label and DPLabel.
2024-02-23 13:46:57 +00:00
Orlando Cazalet-Hyams
ababa96475
[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.
2024-02-20 16:00:55 +00:00
Orlando Cazalet-Hyams
a93a4ec7dd Reapply "[DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default"
This reapplies commit bdde5f9 by undoing the revert fd3a0c185f17.

The previous reapplication d759618df763 was reverted due to a crash
(reproducer in comments for d759618df763) which was fixed in #81737.

As noted in the original commit, this commit may break downstream tests.
If this commit is breaking your downstream tests, please see comment 12 in
[0], which documents the kind of variation in tests we'd expect to see from
this change and what to do about it.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
2024-02-14 16:15:38 +00:00
Arthur Eubanks
fd3a0c185f Revert "Reapply "[DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default""
This reverts commit d759618df76361a8e490eeae5c5399e0738cbfd0.

Causes crashes, see comments in d759618df7.
2024-02-13 21:43:38 +00:00
OCHyams
d759618df7 Reapply "[DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default"
This reapplies commit bdde5f9 by undoing the revert bc66e0c.

The previous reapplication 5c9f768 was reverted due to a crash
(reproducer in comments for 5c9f768) which was fixed in #81595.

As noted in the original commit, this commit may break downstream tests.
If this commit is breaking your downstream tests, please see comment 12 in
[0], which documents the kind of variation in tests we'd expect to see from
this change and what to do about it.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
2024-02-13 08:51:57 +00:00
Arthur Eubanks
bc66e0cf9f Revert "Reapply "[DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default""
This reverts commit 5c9f7682b090124d9a8b69f92d3f7c269dca25fc.

Causes crashes, see comments on 5c9f7682b0.
2024-02-13 00:57:28 +00:00
Stephen Tozer
5c9f7682b0 Reapply "[DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default"
This reapplies commit bdde5f9bea75e897bcc31a95b9c3376988c211cc.

The above commit previously failed due to buildbot errors:
  https://lab.llvm.org/buildbot/#/builders/205/builds/25126
  https://lab.llvm.org/buildbot/#/builders/184/builds/10242

These failures should have been respectively resolved by the commits:
  afa413a132c0959295df36c28814ee83948e4931
  b5a273a1cfe6f509f8d2541e04d9186438f33348

As noted in the original commit, this commit may break downstream tests.
If this commit is breaking your downstream tests, please see comment 12 in
[0], which documents the kind of variation in tests we'd expect to see from
this change and what to do about it.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
2024-02-12 15:49:27 +00:00
Jeremy Morse
13c14ad42c Revert "[DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default"
This reverts commit bdde5f9bea75e897bcc31a95b9c3376988c211cc.

Two situations that are tripping a few buildbots:

  https://lab.llvm.org/buildbot/#/builders/205/builds/25126

Here, polly is currently presenting a DebugLoc attached to a debugging
intrinsic as a "true" source location in a user report, something that's
unreliable.

  https://lab.llvm.org/buildbot/#/builders/184/builds/10242

These HWAsan failures are probably (97% confidence) because in
StackInfoBuilder::visit we're not observing DPValues attached to lifetime
intrinsics because they're delt with higher up the function.

But it's late-o'clock here, so revert for now.
2024-02-08 18:20:37 +00:00
Jeremy Morse
bdde5f9bea [DebugInfo][RemoveDIs] Turn on non-instrinsic debug-info by default
This patch causes all variable-location debug-info to be converted into
non-intrinsic records as they passes through the optimisation /
instrumentation passes. There's a brief introduction here [0] and a more
detailed thread on what this means on discourse at [1].

If this commit is breaking your downstream tests, please see comment 12 in
[1], which documents the kind of variation in tests we'd expect to see from
this change and what to do about it.

[0] https://llvm.org/docs/RemoveDIsDebugInfo.html
[1] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
2024-02-08 17:05:09 +00:00
Jeremy Morse
1a42b3804f
[DebugInfo][RemoveDIs] Erase ranges of instructions individually (#81007)
The BasicBlock::erase method simply removes a range of instructions from
the instlist by unlinking them. However, now that we're attaching
debug-info directly to instructions, some cleanup is required, so use
eraseFromParent on each instruction instead.

This is less efficient, but rare, and seemingly only WASM EH Prepare
uses this method of BasicBlock. Detected via a memory leak check in
asan.

(asan is always the final boss for whatever I do).
2024-02-08 10:27:34 +00:00
Jeremy Morse
ddc493579f [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).
2024-02-06 13:29:56 +00:00
Jeremy Morse
157b62612a Revert "[DebugInfo][RemoveDIs] Don't pointlessly scan funcs for debug-info (#79327)"
This reverts commit c23608b8d58bdeb0134d99168e6d0335da2c8366.

It looks like this depends on #79345, which isn't going to land today, so revert for now.
2024-01-26 14:53:07 +00:00
Jeremy Morse
c23608b8d5
[DebugInfo][RemoveDIs] Don't pointlessly scan funcs for debug-info (#79327)
The utility functions this patch modifies are part of cleanly
transitioning from a context where we use dbg.value intrinsics to one
where we use DPValue objects to record debug-info, and back again.
However, this is a waste of time in non-debug builds (i.e. no -g on the
command line). We still have to call the function on all blocks though
to set the IsNewDbgInfoFormat flag.

To reduce the overhead of this, test whether there's any debug-info in
the function by checking whether the function has a DISubprogram, and
pass a flag down to the utility functions indicating whether they can
skip the scan.

It feels a bit dumb to me now that we're scanning and setting a flag in
a load of blocks when we don't have to -- however it's been really
valuable during development for working out where spurious dbg.value
intrinsics leak into a RemoveDIs context. Happily we'll be able to just
delete this flag entirely when RemoveDIs lands and sticks, and the
conversion routines will eventually be pushed down into the debug-info
autoupgrade path.
2024-01-26 09:53:05 +00:00
Jeremy Morse
7fc2592823 [DebugInfo][RemoveDIs] "Final" cleanup for non-instr debug-info (#79121)
Here's a raft of minor fixes for the RemoveDIs project that's replacing
dbg.value intrinsics with DPValue objects, all IMO trivial:
 * When inserting functions or blocks and calling setIsNewDbgInfoFormat,
   do that after setting the Parent pointer, just in case conversion from
   (or to) dbg.value mode is triggered.
 * When transferring DPValues from an empty range in a splice call, don't
   transfer if there are no DPValues attached to the source block at all.
 * stripNonLineTableDebugInfo should drop DPValues.
 * In insertBefore, don't try to transfer DPValues if there aren't any.
2024-01-23 22:52:47 +00:00
Stephen Tozer
d3a6a90ae5
[RemoveDIs][DebugInfo] Enable creation of DPVAssigns, update outstanding AT tests (#79148)
This is the final patch for DPVAssign support, implementing the actual
creation of DPVAssigns and allowing them to be converted along with
dbg.values and dbg.declares. Numerous tests landed in previous patches
will no longer be rotten after this patch lands (previously they would
trivially pass due to DPVAssigns not actually being used), and a further
batch of tests have been added here that require the changes in this
patch before they pass.
2024-01-23 16:38:49 +00:00
Orlando Cazalet-Hyams
fd8fa31c55
[RemoveDIs] Update Coroutine passes to handle DPValues (#74480)
As part of the RemoveDIs project, transitioning to non-instruction debug
info, all debug intrinsic handling code needs to be duplicated to handle
DPValues.

--try-experimental-debuginfo-iterators enables the new debug mode in
tests if the CMake option has been enabled.

`getInsertPtAfterFramePtr` now returns an iterator so we don't lose
debug-info-communicating bits.

---

Depends on #73500, #74090, #74091.
2023-12-13 12:34:37 +00:00
Orlando Cazalet-Hyams
bdbc2db536
[RemoveDIs] Enable conversion from dbg.declare to DPValue (#74090)
Note that all the patches that implement support for declare-style
DPValues have tests that are "rotten green" test without this
patch (i.e., they pass at the moment without testing what we
want them to test). See the Pull Request for more detail on this.
2023-12-13 11:20:53 +00:00
Jeremy Morse
2a95d47ed5 [DebugInfo] Follow up to 34cdc91321 to fix a crash
We're removing trailing debug-records at the correct time, but from the
wrong block. Broken the iterators buildbot:

  https://lab.llvm.org/buildbot/#/builders/275/builds/1889
2023-12-05 16:56:22 +00:00
Jeremy Morse
71809cfc7a
[DebugInfo][RemoveDIs] Avoid leaking trailing DPMarkers (#74458)
In the debug-info-splice implementation, we need to be careful to delete
trailing DPMarkers from blocks when we splice their contents out. This
is equivalent to removing the terminator from a block, then splicing the
rest of it's contents to another block: any DPValues trailing at the end
of the block get moved and we need to clean up afterwards.
2023-12-05 15:41:00 +00:00
Jeremy Morse
37f2f48c8f [DebugInfo][RemoveDIs] Handle a debug-info splicing corner case (#73810)
A large amount of complexity when it comes to shuffling DPValue objects
around is pushed into BasicBlock::spliceDebugInfo, and it gets
comprehensive testing there via the unit tests. It turns out that there's a
corner case though: splicing instructions and debug-info to the end()
iterator requires blocks of DPValues to be concatenated, but the DPValues
don't behave normally as they're dangling at the end of a block. While this
splicing-to-an-empty-block case is rare, and it's even rarer for it to
contain debug-info, it does happen occasionally.

Fix this by wrapping spliceDebugInfo with an outer layer that removes any
dangling DPValues in the destination block -- that way the main splicing
function (renamed to spliceDebugInfoImpl) doesn't need to worry about that
scenario. See the diagram in the added function for more info.
2023-12-01 19:31:27 +00:00
Jeremy Morse
3ef98bcd46 [DebugInfo][RemoveDIs] Support maintaining DPValues in CodeGenPrepare (#73660)
CodeGenPrepare needs to support the maintenence of DPValues, the
non-instruction replacement for dbg.value intrinsics. This means there are
a few functions we need to duplicate or replicate the functionality of:
 * fixupDbgValue for setting users of sunk addr GEPs,
 * The remains of placeDbgValues needs a DPValue implementation for sinking
 * Rollback of RAUWs needs to update DPValues
 * Rollback of instruction removal needs supporting (see github #73350)
 * A few places where we have to use iterators rather than instructions.

There are three places where we have to use the setHeadBit call on
iterators to indicate which portion of debug-info records we're about to
splice around. This is because CodeGenPrepare, unlike other optimisation
passes, is very much concerned with which block an operation occurs in and
where in the block instructions are because it's preparing things to be in
a format that's good for SelectionDAG.

There isn't a large amount of test coverage for debuginfo behaviours in
this pass, hence I've added some more.
2023-11-30 15:29:05 +00:00
Jeremy Morse
2ec0283c04
[DebugInfo][RemoveDIs] Emulate inserting insts in dbg.value sequences (#73350)
Here's a problem for the RemoveDIs project to make debug-info not be
stored in instructions -- in the following sequence:
    dbg.value(foo
    %bar = add i32 ...
    dbg.value(baz
It's possible for rare passes (only CodeGenPrepare) to remove the add
instruction, and then re-insert it back in the same place. When
debug-info is stored in instructions and there's a total order on "when"
things happen this is easy, but by moving that information out of the
instruction stream we start having to do manual maintenance.

This patch adds some utilities for re-inserting an instruction into a
sequence of DPValue objects. Someday we hope to design this away, but
for now it's necessary to support all the things you can do with
dbg.values. The two unit tests show how DPValues get shuffled around
using the relevant function calls. A follow-up patch adds
instrumentation to CodeGenPrepare.
2023-11-30 12:41:52 +00:00
Jeremy Morse
2f7c050e19 [DebugInfo][RemoveDIs] Allow speculative-DPMarker creation
There's good justification for having a function specifying "I need there to be
a marker here, so return the marker there or create a new one". This was going
to come later in the series, but it's starting to become necessary much eariler
alas.

Make use of it in spliceDebugInfo, where we can occasionally splice DPValues
onto the end() iterator of a block while it's been edited.
2023-11-24 15:17:32 +00:00
Jeremy Morse
80d3a4c39f
[DebugInfo][RemoveDIs] Add local-utility plumbing for DPValues (#72276)
This patch re-implements a variety of debug-info maintenence functions
to use DPValues instead of DbgValueInst's: supporting the "new"
non-intrinsic representation of debug-info. As per [0], we need to have
parallel implementations of various utilities for a time, and these are
the most fundamental utilities used throughout the compiler.

I've added --try-experimental-debuginfo-iterators to a variety of RUN
lines: this is a flag that turns on "new debug-info" if it's built into
LLVM, and not otherwise. This should ensure that we have the same
behaviour for the same IR inputs, but using a different internal
representation. For the most part these changes affect SROA/Mem2Reg
promotion of dbg.declares into dbg.value intrinsics (now DPValues),
we're leaving dbg.declares as instructions until later in the day.
There's also some salvaging changes made.

I believe the tests that I've added cover almost all the code being
updated here. The only thing I'm not confident about is SimplifyCFG,
which calls rewriteDebugUsers down a variety of code paths. Those
changes can't immediately get full coverage as an additional patch is
needed that updates handling of Unreachable instructions, will upload
that shortly.

[0]
https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939/9
2023-11-20 16:56:31 +00:00
Jeremy Morse
10a9e7442c [DebugInfo][RemoveDIs] Add conversion utilities for new-debug-info format
This patch plumbs the command line --experimental-debuginfo-iterators flag
in to the pass managers, so that modules can be converted to the new
format, passes run, then converted back to the old format. That allows
developers to test-out the new debuginfo representation across some part of
LLVM with no further work, and from the command line. It also installs
flag-catchers at the various points that bitcode and textual IR can egress
from a process, and temporarily convert the module to dbg.value format when
doing so.

No tests alas as it's designed to be transparent.

Differential Revision: https://reviews.llvm.org/D154372
2023-11-09 22:30:49 +00:00
Jeremy Morse
b002b38fd9 [DebugInfo][RemoveDIs] Add new behind-the-scenes plumbing for debug-info
This is the "central" patch to the removing-debug-intrinsics project: it
changes the instruction movement APIs (insert, move, splice) to interpret
the "Head" bits we're attaching to BasicBlock::iterators, and updates
debug-info records in the background to preserve the ordering of debug-info
(which is in DPValue objects instead of dbg.values). The cost is the
complexity of this patch, plus memory. The benefit is that LLVM developers
can cease thinking about whether they're moving debug-info or not, because
it'll happen behind the scenes.

All that complexity appears in BasicBlock::spliceDebugInfo, see the diagram
there for how we now manually shuffle debug-info around. Each potential
splice configuration gets tested in the added unit tests.

The rest of this patch applies the same reasoning in a variety of
scenarios. When moveBefore (and it's siblings) are used to move
instructions around, the caller has to indicate whether they intend for
debug-info to move too (is it a "Preserving" call or not), and then the
"Head" bits used to determine where debug-info moves to. Similar reasoning
is needed for insertBefore.

Differential Revision: https://reviews.llvm.org/D154353
2023-11-09 15:25:39 +00:00
Jeremy Morse
f1b0a54451 Reapply 7d77bbef4ad92, adding new debug-info classes
This reverts commit 957efa4ce4f0391147cec62746e997226ee2b836.

Original commit message below -- in this follow up, I've shifted
un-necessary inclusions of DebugProgramInstruction.h into being forward
declarations (fixes clang-compile time I hope), and a memory leak in the
DebugInfoTest.cpp IR unittests.

I also tracked a compile-time regression in D154080, more explanation
there, but the result of which is hiding some of the changes behind the
EXPERIMENTAL_DEBUGINFO_ITERATORS compile-time flag. This is tested by the
"new-debug-iterators" buildbot.

[DebugInfo][RemoveDIs] Add prototype storage classes for "new" debug-info

This patch adds a variety of classes needed to record variable location
debug-info without using the existing intrinsic approach, see the rationale
at [0].

The two added files and corresponding unit tests are the majority of the
plumbing required for this, but at this point isn't accessible from the
rest of LLVM as we need to stage it into the repo gently. An overview is
that classes are added for recording variable information attached to Real
(TM) instructions, in the form of DPValues and DPMarker objects. The
metadata-uses of DPValues is plumbed into the metadata hierachy, and a
field added to class Instruction, which are all stimulated in the unit
tests. The next few patches in this series add utilities to convert to/from
this new debug-info format and add instruction/block utilities to have
debug-info automatically updated in the background when various operations
occur.

This patch was reviewed in Phab in D153990 and D154080, I've squashed them
together into this commit as there are dependencies between the two
patches, and there's little profit in landing them separately.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
2023-11-08 16:42:35 +00:00
Jeremy Morse
957efa4ce4 Revert "[DebugInfo][RemoveDIs] Add prototype storage classes for "new" debug-info"
And some intervening fixups. There are two remaining problems:
 * A memory leak via https://lab.llvm.org/buildbot/#/builders/236/builds/7120/steps/10/logs/stdio
 * A performance slowdown with -g where I'm not completely sure what the cause it

These might be fairly straightforwards to fix, but it's the end of the day
hear, so I figure I'll clear the buildbots til tomorrow.

This reverts commit 7d77bbef4ad9230f6f427649373fe46a668aa909.
This reverts commit 9026f35afe6ffdc5e55b6615efcbd36f25b11558.
This reverts commit d97b2b389a0e511c65af6845119eb08b8a2cb473.
2023-11-02 17:41:36 +00:00
Jeremy Morse
7d77bbef4a [DebugInfo][RemoveDIs] Add prototype storage classes for "new" debug-info
This patch adds a variety of classes needed to record variable location
debug-info without using the existing intrinsic approach, see the rationale
at [0].

The two added files and corresponding unit tests are the majority of the
plumbing required for this, but at this point isn't accessible from the
rest of LLVM as we need to stage it into the repo gently. An overview is
that classes are added for recording variable information attached to Real
(TM) instructions, in the form of DPValues and DPMarker objects. The
metadata-uses of DPValues is plumbed into the metadata hierachy, and a
field added to class Instruction, which are all stimulated in the unit
tests. The next few patches in this series add utilities to convert to/from
this new debug-info format and add instruction/block utilities to have
debug-info automatically updated in the background when various operations
occur.

This patch was reviewed in Phab in D153990 and D154080, I've squashed them
together into this commit as there are dependencies between the two
patches, and there's little profit in landing them separately.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
2023-11-02 12:44:53 +00:00
Jeremy Morse
088d272e83 [ADT][DebugInfo][RemoveDIs] Add extra bits to ilist_iterator for debug-info
...behind an experimental CMAKE option that's off by default.

This patch adds a new ilist-iterator-like class that can carry two extra bits
as well as the usual node pointer. This is part of the project to remove
debug-intrinsics from LLVM: see the rationale here [0], they're needed to
signal whether a "position" in a BasicBlock includes any debug-info before or
after the iterator.

This entirely duplicates ilist_iterator, attempting re-use showed it to be a
false economy. It's enable-able through the existing ilist_node options
interface, hence a few sites where the instruction-list type needs to be
updated. The actual main feature, the extra bits in the class, aren't part of
the class unless the cmake flag is given: this is because there's a
compile-time cost associated with it, and I'd like to get everything in-tree
but off-by-default so that we can do proper comparisons.

Nothing actually makes use of this yet, but will do soon, see the Phab patch
stack.

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

Differential Revision: https://reviews.llvm.org/D153777
2023-10-17 15:24:44 +01:00
Jeremy Morse
6942c64e81 [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
2023-09-11 11:48:45 +01:00
Nikita Popov
4eafc9b6ff [IR] Treat callbr as special terminator (PR64215)
isLegalToHoistInto() currently return true for callbr instructions.
That means that a callbr with one successor will be considered a
proper loop preheader, which may result in instructions that use
the callbr return value being hoisted past it.

Fix this by adding callbr to isExceptionTerminator (with a rename
to isSpecialTerminator), which also fixes similar assumptions in
other places.

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

Differential Revision: https://reviews.llvm.org/D158609
2023-08-25 09:20:18 +02:00
Arthur Eubanks
3e39cfe5b4 Revert "Revert "InstSimplify: Require instruction be parented""
This reverts commit 0c03f48480f69b854f86d31235425b5cb71ac921.

Going to fix forward size regression instead due to more dependent patches needing to be reverted otherwise.
2023-06-16 13:53:31 -07:00
Arthur Eubanks
0c03f48480 Revert "InstSimplify: Require instruction be parented"
This reverts commit 1536e299e63d7788f38117b0212ca50eb76d7a3b.

Causes large binary size regressions, see comments on https://reviews.llvm.org/rG1536e299e63d7788f38117b0212ca50eb76d7a3b.
2023-06-16 11:24:29 -07:00
Alan Zhao
d6b4f6786b Revert "Revert "InstSimplify: Require instruction be parented""
This reverts commit 00264eac4d0938ae8a0826da38e4777be269124c.

Reason: caused a bunch of bots to break
2023-06-16 10:58:54 -07:00