A lot of time in LiveDebugValues is spent computing DenseMap keys for
DebugVariables, and they're made up of three pointers, so are large.
This patch installs an index for them: for the SSA and value-to-location
mapping parts of InstrRefBasedLDV we don't need to access things like
the variable declaration or the inlining site, so just use a uint32_t
identifier for each variable fragment that's tracked. The compile-time
performance improvements are substantial (almost 0.4% on the tracker).
About 80% of this patch is just replacing DebugVariable references with
DebugVariableIDs instead, however there are some larger consequences. We
spend lots of time fetching DILocations when emitting DBG_VALUE
instructions, so index those with the DebugVariables: this means all
DILocations on all new DBG_VALUE instructions will normalise to the
first-seen DILocation for the variable (which should be fine).
We also used to keep an ordering of when each variable was seen first in
a DBG_* instruction, in the AllVarsNumbering collection, so that we can
emit new DBG_* instructions in a stable order. We can hang this off the
DebugVariable index instead, so AllVarsNumbering is deleted.
Finally, rather than ordering by AllVarsNumbering just before DBG_*
instructions are linked into the output MIR, store instructions along
with their DebugVariableID, so that they can be sorted by that instead.
When resolving value-numbers to specific machine locations in the final
stages of LiveDebugValues, we've been producing a DenseMap containing
all the value-numbers we're interested in. However we never modify the
map keys as they're all pre-known. Thus, this is a suitable collection
to switch to a sorted vector that gets searched, rather than a DenseMap
that gets probed. The overall operation of LiveDebugValues isn't
affected at all.
This patch adjusts how some data is stored to avoid a number of
un-necessary DenseMap queries. There's no change to the compiler
behaviour, and it's measurably faster on the compile time tracker.
The BlockOrders vector in buildVLocValueMap collects the blocks over
which a variables value have to be determined: however the Cmp ordering
function makes two DenseMap queries to determine the RPO-order of blocks
being compared. And given that sorting is O(N log(N)) comparisons this
isn't fast. So instead, fetch the RPO-numbers of the block collection,
order those, and then map back to the blocks themselves.
The OrderToBB collection mapped an RPO-number to an MBB: it's completely
un-necessary to have DenseMap here, we can just use the RPO number as an
array index. Switch it to a SmallVector and deal with a few consequences
when iterating.
(And for good measure I've jammed in a couple of reserve calls).
Summary:
- Remove wrappers in `MachineDominatorTree`.
- Remove `MachineDominatorTree` update code in
`MachineBasicBlock::SplitCriticalEdge`.
- Use `MachineDomTreeUpdater` in passes which call
`MachineBasicBlock::SplitCriticalEdge` and preserve
`MachineDominatorTreeWrapperPass` or CFG analyses.
Commit abea99f65a97248974c02a5544eaf25fc4240056 introduced related
methods in 2014. Now we have SemiNCA based dominator tree in 2017 and
dominator tree updater, the solution adopted here seems a bit outdated.
This is part of #70452 that changes the type used for the external
interface of MMO to LocationSize as opposed to uint64_t. This means the
constructors take LocationSize, and convert ~UINT64_C(0) to
LocationSize::beforeOrAfter(). The getSize methods return a
LocationSize.
This allows us to be more precise with unknown sizes, not accidentally
treating them as unsigned values, and in the future should allow us to
add proper scalable vector support but none of that is included in this
patch. It should mostly be an NFC.
Global ISel is still expected to use the underlying LLT as it needs, and
are not expected to see unknown sizes for generic operations. Most of
the changes are hopefully fairly mechanical, adding a lot of getValue()
calls and protecting them with hasValue() where needed.
Commit 1b531d54f623 (#74203) removed the usage of unique_ptrs of arrays
in favour of using vectors, but inadvertently increased peak memory
usage by removing the ability to deallocate vector memory that was no
longer needed mid-LDV.
In that same review, it was pointed out that `FuncValueTable` typedef
could be removed, since it was "just a vector".
This commit addresses both issues by making `FuncValueTable` a real data
structure, capable of mapping BBs to ValueTables and able to free
ValueTables as needed.
This reduces peak memory usage in the compiler by 10% in the benchmarks
flagged by the original review.
As a consequence, we had to remove a handful of instances of the
"declare-then-initialize" antipattern in unittests, as the
FuncValueTable class is no longer default-constructible.
These are usually difficult to reason about, and they were being used to
pass raw pointers around with array semantic (i.e., we were using
operator [] on raw pointers). To put it in InstrRef terminology: we were
passing a pointer to a ValueTable but using it as if it were a
FuncValueTable.
These could have easily been SmallVectors, which now allow us to have
reference semantics in some places, as well as simpler initialization.
In the future, we can use even more pass-by-reference with some extra
changes in the code.
This is a conservative workaround for broken liveness tracking of
SUBREG_TO_REG to speculatively fix all targets. The current reported
failures are on X86 only, but this issue should appear for all targets
that use SUBREG_TO_REG. The next minimally correct refinement would be
to disallow only implicit defs.
The coalescer now introduces implicit-defs of the super register to
track the dependency on other subregisters. If we see such an implicit
operand, we cannot simply treat the subregister def as the result
operand in case downstream users depend on the implicitly defined
parts. Really target implementations should be considering the
implicit defs and trying to interpret them appropriately (maybe with
some generic helpers). The full implicit def could possibly be
reported as the move result, rather than the subregister def but that
requires additional work.
Hopefully fixes#64060 as well.
This needs to be applied to the release branch.
https://reviews.llvm.org/D156346
Fixes: https://github.com/llvm/llvm-project/issues/62725
This patch fixes an error in which a DBG_INSTR_REF referring to a DBG_PHI in a
block that is not directly reachable from the entry block results in a crash
during LiveDebugValues. Note that this fix prevents a crash from occurring, but
will give undef locations to users of these PHIs even if a valid location exists.
Reviewed By: jmorse
Differential Revision: https://reviews.llvm.org/D150707
With D68945, more DBG_VALUEs were created without the indirect operand,
instead relying on OP_deref to accomplish the same effect.
At the time, however, we were not able to handle arbitrary expressions
in combination with OP_LLVM_entry_value, so D71416 prevented the use of
such operation in the presence of expressions.
As per the comment in DIExpression::isValid, "we support only entry
values of a simple register location." As such, a simple deref operation
should be supported. In fact, D80345 added support for indirect
DBG_VALUEs.
Taken the patches above into consideration, this commit relaxes the
restrictions on which expressions are allowed for entry value
candidates: the expression must be either empty or a single dereference
operator.
This patch is useful for D141381, which adds support for storing the
address of ABI-indirect parameters on the stack.
Depends on D142160
Differential Revision: https://reviews.llvm.org/D142654
Following support from the previous patches in this stack being added for
variadic DBG_INSTR_REFs to exist, this patch modifies LiveDebugValues to
handle those instructions. Support already exists for DBG_VALUE_LISTs, which
covers most of the work needed to handle these instructions; this patch only
modifies the transferDebugInstrRef function to correctly track them.
Reviewed By: jmorse
Differential Revision: https://reviews.llvm.org/D133927
Prior to this patch, variadic DIExpressions (i.e. ones that contain
DW_OP_LLVM_arg) could only be created by salvaging debug values to create
stack value expressions, resulting in a DBG_VALUE_LIST being created. As of
the previous patch in this patch stack, DBG_INSTR_REF's syntax has been
changed to match DBG_VALUE_LIST in preparation for supporting variadic
expressions. This patch adds some minor changes needed to allow variadic
expressions that aren't stack values to exist, and allows variadic expressions
that are trivially reduceable to non-variadic expressions to be handled
similarly to non-variadic expressions.
Reviewed by: jmorse
Differential Revision: https://reviews.llvm.org/D133926
This patch makes two notable changes to the MIR debug info representation,
which result in different MIR output but identical final DWARF output (NFC
w.r.t. the full compilation). The two changes are:
* The introduction of a new MachineOperand type, MO_DbgInstrRef, which
consists of two unsigned numbers that are used to index an instruction
and an output operand within that instruction, having a meaning
identical to first two operands of the current DBG_INSTR_REF
instruction. This operand is only used in DBG_INSTR_REF (see below).
* A change in syntax for the DBG_INSTR_REF instruction, shuffling the
operands to make it resemble DBG_VALUE_LIST instead of DBG_VALUE,
and replacing the first two operands with a single MO_DbgInstrRef-type
operand.
This patch is the first of a set that will allow DBG_INSTR_REF
instructions to refer to multiple machine locations in the same manner
as DBG_VALUE_LIST.
Reviewed By: jmorse
Differential Revision: https://reviews.llvm.org/D129372
Currently the instruction referencing live debug values has 3 separate
places where we iterate over all known locations to find the best machine
location for a set of values at a given point in the program. Each of these
places has an implementation of this check, and one of them has slightly
different logic to the others. This patch moves the check for the "quality"
of a machine location into a separate function, which also avoids repeatedly
calling expensive functions, giving a slight performance improvement.
Differential Revision: https://reviews.llvm.org/D140412
This patch adds a new function that can be used to check all the
properties, other than the machine values, of a pair of debug values for
equivalence. This is done by folding the "directness" into the
expression, converting the expression to variadic form if it is not
already in that form, and then comparing directly. In a few places which
check whether two debug values are identical to see if their ranges can
be merged, this function will correctly identify cases where two debug
values are expressed differently but have the same meaning, allowing
those ranges to be correctly merged.
Differential Revision: https://reviews.llvm.org/D136173
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
This patch builds on prior support patches to enable support for
variadic debug values in InstrRefLDV, allowing DBG_VALUE_LISTs to
have their ranges extended.
Differential Revision: https://reviews.llvm.org/D128212
This patch adds the last of the changes required to enable
DBG_VALUE_LIST handling in InstrRefLDV, handling variadic debug values
during the transfer tracking step. Most of the changes are fairly
straightforward, and based around tracking multiple locations per
variable in TransferTracker::VLocTracker.
Differential Revision: https://reviews.llvm.org/D128211
In preparation for supporting DBG_VALUE_LIST in InstrRefLDV, this patch
adds the logic for emitting DBG_VALUE_LIST instructions from
InstrRefLDV. The logical changes here are fairly simple, with the main
change being that instead of directly prepending offsets to the DIExpr,
we use appendOpsToArg to modify the expression for individual debug
operands in the expression. The function emitLoc is also changed to take
a list of debug ops, with an empty list meaning an undef value.
Differential Revision: https://reviews.llvm.org/D128209
In preparation for adding support for DBG_VALUE_LIST instructions in
InstrRefLDV, this patch updates the logic for joining variables at block
joins to support joining variables that use multiple debug operands.
This is one of the more meaty "logical" changes, although the line count
isn't too high - this changes pickVPHILoc to find a valid joined
location for every operand, with part of the function being split off
into pickValuePHILoc which finds a location for a single operand.
Differential Revision: https://reviews.llvm.org/D128180
In preparation for allowing InstrRefBasedLDV to handle DBG_VALUE_LIST,
this patch updates the internal representation that it uses to represent
debug values to store a list of values. This is one of the more
significant changes in terms of line count, but is fairly simple and
should not affect the output of this pass.
Differential Revision: https://reviews.llvm.org/D128177
Currently, InstrRefLDV only handles DBG_VALUE instructions, not
DBG_VALUE_LIST, and as a result of this it handles these instructions
using functions that only work for that type of debug value, i.e. using
getOperand(0) to get the debug operand. This patch changes this to use
the generic debug value functions, such as getDebugOperand and
isDebugOffsetImm, as well as adding an IsVariadic field to the
DbgValueProperties class and a few other minor changes to acknowledge
DBG_VALUE_LISTs. Note that this patch does not add support for
DBG_VALUE_LIST here, but is a precursor to other patches that do add
that support.
Differential Revision: https://reviews.llvm.org/D128174
In the InstrRefBasedImpl for LiveDebugValues, we attempt to propagate
debug values through basic blocks in part by checking to see whether all
a variable's incoming debug values to a BB "agree", i.e. whether their
properties match and they refer to the same underlying value.
Prior to this patch, the check for agreement between incoming values
relied on exact equality, which meant that a VPHI and a Def DbgValue
that referred to the same underlying value would be seen as disagreeing.
This patch changes this behaviour to treat them as referring to the same
value, allowing the shared value to propagate into the BB.
Differential Revision: https://reviews.llvm.org/D125953
This is a follow-up patch to D130999. In the test, the MIR contains an
unreachable MBB but the code attempts to look it up in MLocs. This
patch fixes this issue by checking for the default-constructed value.
rdar://97226240
Differential Revision: https://reviews.llvm.org/D131453
The testcase was delta-reduced from an LTO build with sanitizer
coverage and the MIR tail duplication pass caused a machine basic
block to become unreachable in MIR. This caused the MBB to be invisible
to the reverse post-order traversal used to initialize the MBB <->
RPONumber lookup tables.
rdar://97226240
Differential Revision: https://reviews.llvm.org/D130999
llvm::sort is beneficial even when we use the iterator-based overload,
since it can optionally shuffle the elements (to detect
non-determinism). However llvm::sort is not usable everywhere, for
example, in compiler-rt.
Reviewed By: nhaehnle
Differential Revision: https://reviews.llvm.org/D130406
