At this stage I'm just opportunistically deleting any code using
debug-intrinsic types, largely adjacent to calls to findDbgUsers. I'll
get to deleting that in probably one or more two commits.
The "preserve input debug-info format" flag allowed some tooling to opt
into not seeing the new debug records yet, and to not autoupgrade. This
was good at the time, but un-necessary now that we'll be ditching
intrinsics shortly.
It also hides errors now: verify-uselistorder was hardcoding this flag
to on, and as a result it hasn't seen debug records before. Thus, we
missed a uselistorder variation: constant-expressions such as GEPs can
be contained within debug records and completely isolated from the value
hierachy, see the metadata-use-uselistorder.ll test. These Values didn't
get ordered, but were legitimate uses of constants like "i64 0", and we
now run into difficulty handling that. The patch to AsmWriter seeks
Values to order even through debug-info now.
Finally there are a few intrinsics-tests relying on this flag that we
can just delete, such as one in llvm-reduce and another few in the
LocalTest unit tests. For the fast-isel test, it was added in
https://reviews.llvm.org/D67703 explicitly for checking the size of
blocks without debug-info and in 1525abb9c94 the codepath it tests moved
towards being sunsetted. It'll be totally redundant once RemoveDIs is on
permanently.
Note that there's now no explicit test for the textual-IR autoupgrade
path. I submit that we can rely on the thousands of .ll files where
we've only been bothered to update the outputs, not the inputs, to debug
records.
During the transition from debug intrinsics to debug records, we used
several different command line options to customise handling: the
printing of debug records to bitcode and textual could be independent of
how the debug-info was represented inside a module, whether the
autoupgrader ran could be customised. This was all valuable during
development, but now that totally removing debug intrinsics is coming
up, this patch removes those options in favour of a single flag
(experimental-debuginfo-iterators), which enables autoupgrade, in-memory
debug records, and debug record printing to bitcode and textual IR.
We need to do this ahead of removing the
experimental-debuginfo-iterators flag, to reduce the amount of
test-juggling that happens at that time.
There are quite a number of weird test behaviours related to this --
some of which I simply delete in this commit. Things like
print-non-instruction-debug-info.ll , the test suite now checks for
debug records in all tests, and we don't want to check we can print as
intrinsics. Or the update_test_checks tests -- these are duplicated with
write-experimental-debuginfo=false to ensure file writing for intrinsics
is correct, but that's something we're imminently going to delete.
A short survey of curious test changes:
* free-intrinsics.ll: we don't need to test that debug-info is a zero
cost intrinsic, because we won't be using intrinsics in the future.
* undef-dbg-val.ll: apparently we pinned this to non-RemoveDIs in-memory
mode while we sorted something out; it works now either way.
* salvage-cast-debug-info.ll: was testing intrinsics-in-memory get
salvaged, isn't necessary now
* localize-constexpr-debuginfo.ll: was producing "dead metadata"
intrinsics for optimised-out variable values, dbg-records takes the
(correct) representation of poison/undef as an operand. Looks like we
didn't update this in the past to avoid spurious test differences.
* Transforms/Scalarizer/dbginfo.ll: this test was explicitly testing
that debug-info affected codegen, and we deferred updating the tests
until now. This is just one of those silent gnochange issues that get
fixed by RemoveDIs.
Finally: I've added a bitcode test, dbg-intrinsics-autoupgrade.ll.bc,
that checks we can autoupgrade debug intrinsics that are in bitcode into
the new debug records.
Reapplies commit c5aeca73 (and its followup commit 21396be8), which were
reverted due to missing functionality in MLIR and Flang regarding printing
debug records. This has now been added in commit 08aa511, along with support
for printing debug records in flang.
This reverts commit 2dc2290860355dd2bac3b655eea895fe30fde257.
"[Flang] Update test to not check for tail calls on debug intrinsics" &
"Reapply#3 "[RemoveDIs] Load into new debug info format by default in LLVM (#89799)"
Recent updates to flang have added debug info generation via MLIR, a path
which currently does not support debug records. The patch that enables
debug records by default (and a small followup patch) are thus being
reverted until the MLIR path has been fixed.
This reverts commits:
21396be865b4640abf6afa0b05de6708a1a996e0
c5aeca732d1ff6769b0659efebd1cfb5f60487e4
Reapplies commit 91446e2, which was reverted due to a downstream error,
discussed on the pull request. The error could not be reproduced
upstream, and cannot be reproduced downstream as-of current main, so
until the error can be confirmed to still exist this patch should
return.
This reverts commit 23f8fac745bdde70ed4f9c585d19c4913734f1b8.
This patch uses `DIExpression::foldConstantMath()` at the result of a
Salvaged expression, that is, it runs the folding optimizations after an
expression has been salvaged completely, to reduce how many times the
fold optimization function is called. Which should help in reducing the
size of DIExpressions that grow because of salvaging debug info
After checking the size of the dSYM with and without this change, I saw
a decrease of about 300KB, where the debug_loc section is about 1.6 GB
in size.
Where the debug loc section reduced in size by 212KB and it is 193MB in
size, the rest comes from the debug_info section
This is part of a stack of patches and comes after:
https://github.com/llvm/llvm-project/pull/69768https://github.com/llvm/llvm-project/pull/71717https://github.com/llvm/llvm-project/pull/71718https://github.com/llvm/llvm-project/pull/71719
This reverts commit 91446e2aa687ec57ad88dc0df793d0c6e694a7c9 and
a unittest followup 1530f319311908b06fe935c89fca692d3e53184f (#90476).
In a stage-2 -flto=thin -gsplit-dwarf -g -fdebug-info-for-profiling
-fprofile-sample-use= build of clang, a ThinLTO backend compile has
assertion failures:
Global is external, but doesn't have external or weak linkage!
ptr @_ZN5clang12ast_matchers8internal18makeAllOfCompositeINS_8QualTypeEEENS1_15BindableMatcherIT_EEN4llvm8ArrayRefIPKNS1_7MatcherIS5_EEEE
function declaration may only have a unique !dbg attachment
ptr @_ZN5clang12ast_matchers8internal18makeAllOfCompositeINS_8QualTypeEEENS1_15BindableMatcherIT_EEN4llvm8ArrayRefIPKNS1_7MatcherIS5_EEEE
The failures somehow go away if -fprofile-sample-use= is removed.
This patch updates the unittests that can be changed to the new format
after #89799 (which changes the default format everywhere) to avoid a
loss in coverage for the (new) default debug info format.
I'm planning to remove StringRef::equals in favor of
StringRef::operator==.
- StringRef::operator==/!= outnumber StringRef::equals by a factor of
31 under llvm/ in terms of their usage.
- The elimination of StringRef::equals brings StringRef closer to
std::string_view, which has operator== but not equals.
- S == "foo" is more readable than S.equals("foo"), especially for
!Long.Expression.equals("str") vs Long.Expression != "str".
Reapplies the original commit:
2f01fd99eb8c8ab3db9aba72c4f00e31e9e60a05
The previous application of this patch failed due to some missing
DbgVariableRecord support in clang, which has been added now by commit
8805465e.
This will probably break some downstream tools that don't already handle
debug records. If your downstream code breaks as a result of this
change, the simplest fix is to convert the module in question to the old
debug format before you process it, using
`Module::convertFromNewDbgValues()`. For more information about how to
handle debug records or about what has changed, see the migration
document:
https://llvm.org/docs/RemoveDIsDebugInfo.html
This reverts commit 4fd319ae273ed6c252f2067909c1abd9f6d97efa.
Fixes the broken tests in the original commit:
2f01fd99eb8c8ab3db9aba72c4f00e31e9e60a05
This will probably break some downstream tools that don't already handle
debug records. If your downstream code breaks as a result of this
change, the simplest fix is to convert the module in question to the old
debug format before you process it, using
`Module::convertFromNewDbgValues()`. For more information about how to
handle debug records or about what has changed, see the migration
document:
https://llvm.org/docs/RemoveDIsDebugInfo.html
This reverts commit 00821fed09969305b0003d3313c44d1e761a7131.
This patch enables parsing and creating modules directly into the new
debug info format. Prior to this patch, all modules were constructed
with the old debug info format by default, and would be converted into
the new format just before running LLVM passes. This is an important
milestone, in that this means that every tool will now be exposed to
debug records, rather than those that run LLVM passes. As far as I've
tested, all LLVM tools/projects now either handle debug records, or
convert them to the old intrinsic format.
There are a few unit tests that need updating for this patch; these are
either cases of tests that previously needed to set the debug info
format to function, or tests that depend on the old debug info format in
some way. There should be no visible change in the output of any LLVM
tool as a result of this patch, although the likelihood of this patch
breaking downstream code means an NFC tag might be a little misleading,
if not technically incorrect:
This will probably break some downstream tools that don't already handle
debug records. If your downstream code breaks as a result of this
change, the simplest fix is to convert the module in question to the old
debug format before you process it, using
`Module::convertFromNewDbgValues()`. For more information about how to
handle debug records or about what has changed, see the migration
document:
https://llvm.org/docs/RemoveDIsDebugInfo.html
In the debug intrinsic class heirachy, a dbg.assign is a (inherits from)
dbg.value, so `findDbgValues` returns dbg.values and dbg.assigns (by
design). That hierarchy doesn't exist for DbgRecords - fix findDbgValues
to return dbg_assign records as well as dbg_values and add unittest.
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.
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.
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.
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
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.
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
Since we no longer support typed LLVM IR pointer types, the code can
be simplified into for example using PointerType::get directly instead
of using Type::getInt8PtrTy and Type::getInt32PtrTy etc.
Differential Revision: https://reviews.llvm.org/D156733
This reverts commit 0c03f48480f69b854f86d31235425b5cb71ac921.
Going to fix forward size regression instead due to more dependent patches needing to be reverted otherwise.
Unlike every other analysis and transform, simplifyInstruction
permitted operating on instructions which are not inserted
into a function. This created an edge case no other code needs
to really worry about, and limited transforms in cases that
can make use of the context function. Only the inliner and a handful
of other utilities were making use of this, so just fix up these
edge cases. Results in some IR ordering differences since
cloned blocks are inserted eagerly now. Plus some additional
simplifications trigger (e.g. some add 0s now folded out that
previously didn't).
A ValueAsMetadata may be replaced with nullptr for several reasons including
deleting (certain) values and value remapping a use-before-def. In the case of
a MetadataAsValue user, handleChangedOperand intercepts and replaces the
metadata with an empty tuple (!{}).
At the moment, an empty metadata operand in a debug intrinsics signals that it
can be deleted.
Given that we end up with empty metadata operands in circumstances where the
Value has been "lost" the current behaviour can lead to incorrect variable
locations. Instead, we should treat empty metadata as meaning "there is no
location for the variable" (the same as we currently treat undef operands).
This patch removes the deletion logic from wouldInstructionBeTriviallyDead.
Related to https://discourse.llvm.org/t/auto-undef-debug-uses-of-a-deleted-value
Reviewed By: StephenTozer
Differential Revision: https://reviews.llvm.org/D140901
The out-param vector from findDbgValues and findDbgUsers should not include
duplicates, which is possible if the debug intrinsic uses the value multiple
times. This filter is already in place for multiple uses in a `DIArgLists`;
extend it to cover dbg.assigns too because a Value may be used in both the
address and value components.
Additionally, refactor the duplicated functionality between findDbgValues and
FindDbgUsers into a new function findDbgIntrinsics.
Reviewed By: jmorse, StephenTozer
Differential Revision: https://reviews.llvm.org/D148788
Clang-format InstructionSimplify and convert all "FunctionName"s to
"functionName". This patch does touch a lot of files but gets done with
the cleanup of InstructionSimplify in one commit.
This is the alternative to the less invasive clang-format only patch: D126783
Reviewed By: spatel, rengolin
Differential Revision: https://reviews.llvm.org/D126889
Based on the output of include-what-you-use.
This is a big chunk of changes. It is very likely to break downstream code
unless they took a lot of care in avoiding hidden ehader dependencies, something
the LLVM codebase doesn't do that well :-/
I've tried to summarize the biggest change below:
- llvm/include/llvm-c/Core.h: no longer includes llvm-c/ErrorHandling.h
- llvm/IR/DIBuilder.h no longer includes llvm/IR/DebugInfo.h
- llvm/IR/IRBuilder.h no longer includes llvm/IR/IntrinsicInst.h
- llvm/IR/LLVMRemarkStreamer.h no longer includes llvm/Support/ToolOutputFile.h
- llvm/IR/LegacyPassManager.h no longer include llvm/Pass.h
- llvm/IR/Type.h no longer includes llvm/ADT/SmallPtrSet.h
- llvm/IR/PassManager.h no longer includes llvm/Pass.h nor llvm/Support/Debug.h
And the usual count of preprocessed lines:
$ clang++ -E -Iinclude -I../llvm/include ../llvm/lib/IR/*.cpp -std=c++14 -fno-rtti -fno-exceptions | wc -l
before: 6400831
after: 6189948
200k lines less to process is no that bad ;-)
Discourse thread on the topic: https://llvm.discourse.group/t/include-what-you-use-include-cleanup
Differential Revision: https://reviews.llvm.org/D118652
D106850 introduced a simplification for llvm.vscale by looking at the
surrounding function's vscale_range attributes. The call that's being
simplified may not yet have been inserted into the IR. This happens for
example during function cloning.
This patch fixes the issue by checking if the instruction is in a
parent basic block.
Changes to function calls in LocalTest resulted in comparisons between
unsigned values and signed literals; the latter have been updated to be
unsigned to prevent this warning.
This patch updates DbgVariableIntrinsics to support use of a DIArgList for the
location operand, resulting in a significant change to its interface. This patch
does not update all IR passes to support multiple location operands in a
dbg.value; the only change is to update the DbgVariableIntrinsic interface and
its uses. All code outside of the intrinsic classes assumes that an intrinsic
will always have exactly one location operand; they will still support
DIArgLists, but only if they contain exactly one Value.
Among other changes, the setOperand and setArgOperand functions in
DbgVariableIntrinsic have been made private. This is to prevent code from
setting the operands of these intrinsics directly, which could easily result in
incorrect/invalid operands being set. This does not prevent these functions from
being called on a debug intrinsic at all, as they can still be called on any
CallInst pointer; it is assumed that any code directly setting the operands on a
generic call instruction is doing so safely. The intention for making these
functions private is to prevent DIArgLists from being overwritten by code that's
naively trying to replace one of the Values it points to, and also to fail fast
if a DbgVariableIntrinsic is updated to use a DIArgList without a valid
corresponding DIExpression.
