Skip to content

Commit

Permalink
[WebAssembly] Redesign DebugValueManager
Browse files Browse the repository at this point in the history
The current `DebugValueManager`, which is mostly used in `RegStackify`,
simply sinks `DBG_VALUE`s along when a def instruction sinks.
(`RegStackify` only does sinks; it doesn't do hoists.)

But this simple strategy can result in incorrect combinations of
variables' values which would have not been possible in the original
program. In this case, LLVM's policy is to make the value unavailable,
so they will be shown as 'optimized out', rather than showing inaccurate
debug info. Especially, when an instruction sinks, its original
`DBG_VALUE` should be set to undef. This is well illustrated in the
third example in
https://llvm.org/docs/SourceLevelDebugging.html#instruction-scheduling.

This CL rewrites `DebugValueManager` with this principle in mind. When
sinking an instruction, it sinks its eligible `DBG_VALUE`s with it, but
also leaves undef `DBG_VALUE`s in the original place to make those
variables' values undefined.

Also, unlike the current version, we sink only an eligible subset of
`DBG_VALUE`s with a def instruction. See comments in the code for
details.

In case of cloning, because the original def is still there, we don't
set its `DBG_VALUE`s to undef. But we clone only an eligible subset of
`DBG_VALUE`s here as well.

One consequence of this change is that now we do sinking and cloning of
the def instruction itself within the `DebugValueManager`'s `sink` and
`clone` methods. This is necessary because the `DebugValueManager` needs
to know the original def's location before sinking and cloning in order
to scan other interfering `DBG_VALUE`s between the original def and the
insertion point. If we want to separate these two, we need to call
`DebugValueManager`'s `sink` and `clone` methods //before//
sinking/cloning the def instruction, which I don't think is a good
design alternative either, because the user of this class needs to pay
extra attention when using it.

Because this change is fixing the existing inaccuracy of the current
debug info, this reduces the variable info coverage in debug info, but
not by a large margin. In Emscripten core benchmarks compiled with
`-O1`, the coverage goes from 56.6% down to 55.2%, which I doubt will be
a noticeable drop. The compilation time doesn't have any meaningful
difference either with this change.

Reviewed By: dschuff

Differential Revision: https://reviews.llvm.org/D146744
  • Loading branch information
aheejin committed Mar 29, 2023
1 parent e0de24c commit 5a55c95
Show file tree
Hide file tree
Showing 7 changed files with 913 additions and 111 deletions.
44 changes: 44 additions & 0 deletions llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,50 @@ inline unsigned GetDefaultP2Align(unsigned Opc) {
return Align;
}

inline bool isConst(unsigned Opc) {
switch (Opc) {
case WebAssembly::CONST_I32:
case WebAssembly::CONST_I32_S:
case WebAssembly::CONST_I64:
case WebAssembly::CONST_I64_S:
case WebAssembly::CONST_F32:
case WebAssembly::CONST_F32_S:
case WebAssembly::CONST_F64:
case WebAssembly::CONST_F64_S:
case WebAssembly::CONST_V128_I8x16:
case WebAssembly::CONST_V128_I8x16_S:
case WebAssembly::CONST_V128_I16x8:
case WebAssembly::CONST_V128_I16x8_S:
case WebAssembly::CONST_V128_I32x4:
case WebAssembly::CONST_V128_I32x4_S:
case WebAssembly::CONST_V128_I64x2:
case WebAssembly::CONST_V128_I64x2_S:
case WebAssembly::CONST_V128_F32x4:
case WebAssembly::CONST_V128_F32x4_S:
case WebAssembly::CONST_V128_F64x2:
case WebAssembly::CONST_V128_F64x2_S:
return true;
default:
return false;
}
}

inline bool isScalarConst(unsigned Opc) {
switch (Opc) {
case WebAssembly::CONST_I32:
case WebAssembly::CONST_I32_S:
case WebAssembly::CONST_I64:
case WebAssembly::CONST_I64_S:
case WebAssembly::CONST_F32:
case WebAssembly::CONST_F32_S:
case WebAssembly::CONST_F64:
case WebAssembly::CONST_F64_S:
return true;
default:
return false;
}
}

inline bool isArgument(unsigned Opc) {
switch (Opc) {
case WebAssembly::ARGUMENT_i32:
Expand Down

0 comments on commit 5a55c95

Please sign in to comment.