Only emit value label aliases for lowered instructions

[philipc]

If we emit a value label alias for an instruction that isn't lowered, then that signals that the value has been optimised out. However, since it is an alias we know that the value also exists in an earlier vreg, so we should skip the alias and use that instead.

This situation occurs often for memory indexes on AArch64. We translate memory stores into instructions such as:

v8 = iconst.i32 42
v9 = uextend.i64 v6
v10 = load.i64 notrap aligned readonly can_move checked v0+56
v11 = iadd v10, v9
v12 = iconst.i64 20
v13 = iadd v11, v12  ; v12 = 20
store little heap v8, v13  ; v8 = 42

Here, v6 is a memory index (which has a label) and v9 is an extension of the memory index (which has a label alias, added by cast_index_to_pointer_ty()). This is lowered to:

 40c:       52800540        mov     w0, #0x2a                       // #42
 410:       f9401c41        ldr     x1, [x2, #56]
 414:       91005021        add     x1, x1, #0x14
 418:       b8384820        str     w0, [x1, w24, uxtw]

The uextend has been folded into the str, so v9 has been optimised out. But v6 is still present in w24, so the debuginfo should use that instead.

This fixes the following tests for AArch64:

native_debug::lldb::dwarf_cold_block
native_debug::lldb::dwarf_fib_wasm
native_debug::lldb::dwarf_fib_wasm_dwarf5
native_debug::lldb::dwarf_fib_wasm_split4
native_debug::lldb::dwarf_fission
native_debug::lldb::dwarf_fraction_norm
native_debug::lldb::dwarf_imported_memory
native_debug::lldb::dwarf_shared_memory
native_debug::lldb::dwarf_simple
native_debug::lldb::dwarf_spilled_frame_base

---

Fixes #12677
Probably fixes other reports of problems on AArch64 since nothing really worked, but I didn't test.

Doesn't fix:

native_debug::lldb::dwarf_generic
native_debug::lldb::dwarf_multiple_codegen_units

This PR contains a more conservative fix than what I gave in #12677 (comment). The fix here only affects label aliases, and the commit message gives my reasoning for why I think this is the correct thing to do for them. I can't convince myself that this would also be a correct change for the original label, and I also don't see a need to.

Tested on macOS AArch64. Also tested native_debug::lldb::dwarf_simple on linux AArch64 running under qemu with its gdb stub. Confirmed tests didn't work before this commit.

[philipc]

This is a bit of a hack that avoids changing get_value_labels. Can do it differently if desired.

[alexcrichton]

Looks and sounds reasonable to me, thanks again @philipc!

As a heads up in the last Wasmtime meeting we discussed the long-term fate of DWARF w.r.t. the upcoming guest-based debugging, and the overall consensus is that we're likely to remove most of this infrastructure eventually once guest-based debugging is on par with expected experience. No immediate changes necessarily (if you'd like we're happy to cc you on issues), but wanted to make you aware of our longer-term plans too.

[philipc]

My personal stance on this is that it's nice to have an open source project using the write support in gimli; it gives me something to work on to improve that support. But it's not something that I'm likely to use myself, and I don't want to put effort into something that is going to be deleted.

Would this removal only be for the DWARF support, or also the cranelift codegen value location tracking? The latter would be useful for rustc_codegen_cranelift (which is also using gimli's write support), but it's also something that may need a rewrite - only a single place in wasmtime adds a value label alias, and it feels like a workaround for the problem of handling debug info during optimisation, rather than a principled solution.

[cfallin]

Nothing is firmly decided yet, but if we do standardize further on guest-debugging only in Wasmtime, it would not mean that we delete bits of Cranelift used by debugging support in rustc_codegen_cranelift -- that has value on its own and I wouldn't want to regress it. (At least, that's my take, but other Cranelift maintainers please chime in as well...)

[alexcrichton]

Once guest-debugging is cemented I'd personally like to see a state where the most complicated bits of the dwarf transform are removed in favor of guest-debugging, but I'd still like the ability to emit at least bare-bones dwarf maybe with just line tables. That way debuggers would have something, but printing values/etc wouldn't be expected to work.

I'd agree with Chris though that preserving rustc_codegen_cranelift's support should be a goal as well. I don't have a great handle myself on the cost of the current support in terms of maintainership though, for that I'd defer mostly to you @cfallin or maybe @fitzgen if he's got thoughts.

[cfallin]

I don't have a great handle myself on the cost of the current support in terms of maintainership though

The two main places this comes up are in RA2 (separate analysis to compute debug-label locations after allocation completes) and the MachInst infra (the whole value-label framework touched here); in both cases it was a pain to get right (probably added +2-3 weeks to initial development), and that cost would reoccur if we were ever to do a large refactor or replacement (e.g. a new regalloc), but right now at least it's "dormant" and doesn't impact our mainline work on new instruction lowerings / mid-end opts / etc.

[tschneidereit]

We discussed this back when the original RFC was pending, and all agreed that we'd retain support for native debugging to support cgclif. For what it's worth, I think backtracking on that commitment would be very regrettable, and that not explicitly deciding to not support inspecting locals would be backtracking on that.

I appreciate that there is a maintenance burden to this, but it feels a bit bait-and-switch to me to change our minds on this at this point. (I unfortunately wasn't able to attend last week's Wasmtime meeting, otherwise I'd have made these points there.)

[cfallin]

Yes, to clarify, I was answering Alex's question about what the maintenance burden is, but (to reiterate my point above) I very much am in favor of retaining debugging support in Cranelift for cg_clif.

The discussion at last week's meeting was about eventually removing Wasmtime's use of native debugging features in favor of guest-debugging only, once support is fully landed and we confirm it meets users' needs. Note that that's separate from the Cranelift-level stuff that supports cg_clif.

[tschneidereit]

Oh yeah, I should clarify (and apologize for making it seem that I meant something else!) that I'm largely reacting to Alex' comment:

I'd personally like to see a state where the most complicated bits of the dwarf transform are removed in favor of guest-debugging, but I'd still like the ability to emit at least bare-bones dwarf maybe with just line tables. That way debuggers would have something, but printing values/etc wouldn't be expected to work.

If that's also meant to affect just Wasmtime, then that, too, seems fine. But if it'd apply to cgclif as well, we should certainly talk more.