[PATCH] D138943: [WebAssembly] Enable LiveDebugValues analysis

Thu Dec 15 23:57:25 PST 2022

aheejin added a comment.

@jmorse Thanks for your review, and sorry for the late reply!

In D138943#3960663 <https://reviews.llvm.org/D138943#3960663>, @jmorse wrote:

> I see from the inline comments that virtual registers in WASM at this stage may or may not be meaningful -- if we can avoid LiveDebugValues having to encounter virtual registers, that's highly preferred, as it avoids considering a whole load of scenarios that LiveDebugValues isn't designed for. The number space for LocIndex is designed only for physregs and spill slots (physregs from 1 to 2^30, spill slots from 2^30+1 to 2^31-1). Adding the number range for virtual registers would require more testing.

I removed all register-based (= dangling) `DBG_VALUE`s for Wasm in D139675 <https://reviews.llvm.org/D139675>; `WebAssemblyDebugFixup` runs right before this analysis. So now we don't have any virtual registers participating here, and I re-enabled the physical register assertion, so we can maintain `LocIndex` only deals with physical registers.

> Stepping back a bit: as ever I'm not very familiar with WASM, but are the "target index" locations mutable? The test added covers a few scenarios that more or less amount to SSA construction, i.e. propagation of the variable locations to successor blocks and merging of locations at PHI points. The main purpose of LiveDebugValues however is to track variable locations/values as they move between phyregs and stack slots, and go out of liveness. If the target index locations are immutable, then you can just get away with using existing SSA construction tools in LLVM to work out how to propagate locations. If they're mutable though, you'll need to put logic in transferRegisterDef and similar to identify when the target-index locations are modified, so that variable locations are terminated correctly.

Thanks for pointing this out. They are mutable, so I should have handled this. There are two kinds of target indices: local indices and stack operands.

1. Locals are something similar to registers in Wasm-land. For local indices, I think we can check for local-defining instructions (`local.set` or `local.tee`).
2. Wasm is a stack machine, so we have a value in certain Wasm value stack location, which changes whenever Wasm instructions produce or consume values. So basically any value-producing instrucion, i.e., instruction with defs, can change values in 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.

I'm working on (1) now. Would you mind if I address it as a follow-up child CL? I disabled `LiveDebugValues` analysis here so this not-yet-fully-implemented analysis doesn't run on Wasm pipeline, and we only check for my basic test cases here.

---

I also have a separate question. How should I handle defs in memory locations? Should I use `SpillLoc`s? (They currently can have only registers as their base though) But looking at this code <https://github.com/llvm/llvm-project/blob/f13d0c125f398745fe38506a6a0b2c9d5e36ec0c/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp#L1568-L1579>, it looks not every store instruction qualifies as a spill. Also you can access the same memory address even with a different register + offset, so it kind of looks all stores can terminate any indirect `DBG_VALUE`s. How do other targets handle defs in indirect `DBG_VALUE` locations?

For Wasm, it looks we have a very small number of indirect `DBG_VALUE`s anyway, so maybe I can start with nullifying them and not dealing with them, as I did for `DBG_VALUE_LIST`s a while ago in D102999 <https://reviews.llvm.org/D102999>. But I'm generally curious about how memory locations should be handled. Sorry for many questions 😅

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138943/new/

https://reviews.llvm.org/D138943