This patch handles virtual registers in the VarLocBasedImpl of the
LiveDebugVariables pass, allowing it to be used on architectures that
depend on virtual registers in debugging, like NVPTX. It enables the
pass for NVPTX.
This replaces some of the most frequent offenders of using a DenseMap that
cause a malloc, where the typical element-count is small enough to fit in
an initial stack allocation.
Most of these are fairly obvious, one to highlight is the collectOffset
method of GEP instructions: if there's a GEP, of course it's going to have
at least one offset, but every time we've called collectOffset we end up
calling malloc as well for the DenseMap in the MapVector.
These would implicitly cast the register to `unsigned`. Switch most of
them to use printReg will give a more readable output. Change some
others to use Register::id() so we can eventually remove the implicit
cast to `unsigned`.
Now revised to actually make the unit test compile, which I'd been
ignoring. No actual functional change, it's a type difference.
Original commit message follows.
[DebugInfo][InstrRef] Index DebugVariables and some DILocations (#99318)
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.
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
Use big obj copy in range for-loop will call copy constructor every time,
which can be avoided by use ref instead.
Reviewed By: skan
Differential Revision: https://reviews.llvm.org/D150024
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
Looks like code assumes that it will be always set, but it's not true:
https://reviews.llvm.org/D150420. This is temporarily suppression to enabled
stricter msan on a bot.
This change initializes the members TSI, LI, DT, PSI, and ORE pointer feilds of the SelectOptimize class to nullptr.
Reviewed By: LuoYuanke
Differential Revision: https://reviews.llvm.org/D148303
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
This adds the missing handling for defs for target index operands, as is
already done for registers.
There are two kinds of target indices: local indices and stack operands.
- Locals are something similar to registers in Wasm-land. For local
indices, we can check for local-defining instructions (`local.set` or
`local.tee`).
- Wasm is a stack machine, so we have values in certain Wasm value stack
location, which change when Wasm instructions produce or consume
values. So basically any value-producing instrucion, i.e., instruction
with defs, can change values in the Wasm stack. But I think we don't
need to worry about this here, because `WebAssemblyDebugFixup`, which
runs right before this analysis, makes sure to insert terminating
`DBG_VALUE $noreg` instructions whenever a stack value gets popped.
After `WebAssemblyDebugFixup`, there shouldn't be any `DBG_VALUE`s for
stack operands that don't have a terminating `DBG_VALUE $noreg` within
the same BB.
So this CL only works on `DBG_VALUE`s for locals. When we encounter a
`local.set` or `local.tee` instructions, we delete `DBG_VALUE`s for
those target index locations from the open range set, so they will not
be availble in `OutLocs`. For example,
```
bb.0:
successors: %bb.1
DBG_VALUE target-index(wasm-local) + 2, $noreg, "var", ...
...
local.set 2 ...
bb.1:
; predecessors: %bb.0
; We shouldn't add `DBG_VALUE target (wasm-local) + 2 here because
; it was killed by 'local.set' in bb.0
```
After disabling register coalescing at -O1, the average PC ranges
covered for Emscripten core benchmarks is currently 20.6% in the LLVM
tot. After applying D138943 and this CL, the coverage goes up to 57%.
This also enables LiveDebugValues analysis in the Wasm pipeline by
default.
Reviewed By: dschuff, jmorse
Differential Revision: https://reviews.llvm.org/D140373
