-
Notifications
You must be signed in to change notification settings - Fork 15k
Description
Bugzilla Link | 50361 |
Version | trunk |
OS | All |
CC | @aardappel,@dschuff |
Extended Description
WebAssemblyDebugValueManager
class
(https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp)
was created when there was no DBG_VALUE_LIST
instruction, so it assumes all
debug value instructions are DBG_VALUE
s, each of which can have at most a single
register operand. WebAssemblyDebugValueManager
groups a register def with its
corresponding DBG_VALUE
s that refer to the def.
- When a reg def moves, we can't move
DBG_VALUE_LIST
s likeDBG_VALUE
s.
With DBG_VALUE_LIST
instruction this relationship does not hold anymore.
DBG_VALUE_LIST
can contain multiple register operands and a complex expression
describing the relationship between those operands. So when moving an
instruction that defines a register, we can move DBG_VALUE
s referring to that
def but can't safely do the same thing with DBG_VALUE_LIST
s. For example, see
the pseudocode below:
%0 = ...
DBG_VALUE %0, ...
%1 = ...
...
%2 = ADD %1, %2
When we move %0 right before %2, we can move the DBG_VALUE
with it:
%1 = ...
...
%0 = ...
DBG_VALUE %0, ...
%2 = ADD %1, %2
But if it is a DBG_VALUE_LIST
, there can be multiple register operands that
aren't supposed to be moved:
%0 = ...
DBG_VALUE_LIST ... %0, %1, ...
%1 = ...
...
%2 = ADD %1, %2
When we move %0 right before %2, we can't move the DBG_VALUE_LIST
in the same
way, because it changes the meaning of the DBG_VALUE_LIST
:
%1 = ...
...
%0 = ...
DBG_VALUE_LIST ... %0, %1, ... ;; Error. %1's value changed by new def
%2 = ADD %1, %2
But currently WebAssemblyDebugValueManager
moves all DBG_VALUE_LIST
s around
in the same way, which should be fixed.
- We can't copy
DBG_VALUE_LIST
s in a brute-force manner as we do for
DBG_VALUE
s.
A single DBG_VALUE_LIST
instruction can contain any number of register
operands. In real world code I have seen a DBG_VALUE_LIST
with dozens of them.
In WebAssemblyRegisterStackify, when we create a tee.local
for a reg def, one
reg def becomes three reg defs, which is well illustrated in this comment:
llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
Lines 606 to 622 in 541c284
/// A multiple-use def in the same block with no intervening memory or register | |
/// dependencies; move the def down, nest it with the current instruction, and | |
/// insert a tee to satisfy the rest of the uses. As an illustration, rewrite | |
/// this: | |
/// | |
/// Reg = INST ... // Def | |
/// INST ..., Reg, ... // Insert | |
/// INST ..., Reg, ... | |
/// INST ..., Reg, ... | |
/// | |
/// to this: | |
/// | |
/// DefReg = INST ... // Def (to become the new Insert) | |
/// TeeReg, Reg = TEE_... DefReg | |
/// INST ..., TeeReg, ... // Insert | |
/// INST ..., Reg, ... | |
/// INST ..., Reg, ... |
(In the old code, there's one def 'Reg', but in the new code, there are three
with the same value: 'Reg', 'DefReg', and 'TeeReg')
And when a def becomes three defs, each corresponding DBG_VALUE
is copied
twice so there will be three copies of it. But if a DBG_VALUE_LIST
contains
dozens of registers and each of them gets tee'd, the DBG_VALUE_LIST
can be
copied exponentially. For example, if a DBG_VALUE_LIST
contains %0, %1, and
%2, after %0 is tee'd, the instruction has 3 copies. And after %1 is tee'd, each
of 3 copies becomes 3 copies, so there will be 9 copies. And after %2 is tee'd
there will be 27 copies.
This way, in real world code, I have seen a BB with ~100 instruction (including
DBG_VALUE
s and DBG_VALUE_LIST
s) becomes several million instructions,
effectively stalling the compilation process. It will look like
infinite-looping.
To fix both bugs, we should come up with a way so that it can handle
DBG_VALUE_LIST
s correctly and efficiently. In the meantime, I am planning to
nullify those DBG_VALUE_LIST
instructions to make them appear as "optimized
out", which is at least correct.