[PATCH] D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass

Wed Dec 19 10:29:28 PST 2018

yurydelendik marked an inline comment as done.
yurydelendik added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/DebugLocEntry.h:24

+struct TargetIndexLocation {
+  int Index;
----------------
dschuff wrote:
> This is very generic and doesn't really say what it's for, or what any of the fields are for. I get that this is trying to be target-independent but maybe it would be easier to understand if it wasn't.  It looks like the index corresponds now to the dwarf register (e.g. to indicate that this is for a local or global), and the offset to indicate which one, but that seems backwards because we use the term "index" to refer to e.g. the index of the global or local. 
> Really what we have might in LLVM terms is a "kind" (i.e. a kind of location, local or stack or global) and an index (which local). "Index" is still a pretty generic term but it does match the usage of the term "index space" in the spec (and I would expect that our numbering should be the same, e.g. sharing the local index space for arguments and local variables).
> 
Here I was trying to us existing MO_TargetIndex "Target-dependent index+offset operand" as it is defined in LLVM (similar to lib/Target/AMDGPU/AMDGPU.h).

I also pondering the idea of using the Offset as "memory" offset by introducing the imaginary array/layout for e.g. locals with indices 0, 1, 2,.. will have offsets +0, +16, +32, ...

The alternative is to define MachineOperandType (e.g. MO_WasmLocation), but it will be more invasive

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52634