[PATCH] D138943: [WebAssembly] Enable LiveDebugValues analysis

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a reviewer: jmorse.
jmorse added a comment.

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.

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.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp:75-79
-  MachineFunctionProperties getRequiredProperties() const override {
-    return MachineFunctionProperties().set(
-        MachineFunctionProperties::Property::NoVRegs);
-  }
-
----------------
Doing this makes sense for WASM, but check that there are no virtual registers remains an important sanity check for other targets -- mixing vregs and physregs would definitely generate broken results. Would you be able to add an assertion somewhere else for everything but WASM? (I don't think this method can know what the target arch is).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138943