[PATCH] D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects.

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added subscribers: debug-info, jmorse.
jmorse added a comment.

(Responding to inline comments) I think the main concern with hard-coding registers in DIExpressions at this stage is that LLVM isn't able to identify when that register is clobbered later on, and terminate the variable location. However, as you say:

> Just to clarify the use-case for AArch64 SVE: ScaleReg is always the same register, namely register 46 which is the DWARF register that contains the value for VG, the number of 64-bit "vector granules " in a scalable vector. During the program VG is constant. A runtime value is needed, because the length of the vector isn't known at compile-time.

To confirm my understanding, 'VG' isn't a conventional "register", but instead it's a target-specific way to signal "whatever the vector scale factor is right now"? Given that it's a runtime constant, I think that resolves the concerns about being a hard-coded value, it doesn't need to be tracked in the same way that conventional registers are.

On the whole, LGTM, but I'd wait a bit to see if @aprantl is happy with the explanation. (Adding debug-info group too).



================
Comment at: llvm/test/CodeGen/AArch64/debug-info-sve-dbg-value.mir:9-10
+
+# CHECK0: : DW_OP_breg31 WSP+8)
+# CHECK0: DW_AT_type {{.*}}ty32
+#
----------------
Could I suggest adding a larger DIExpression as the input to this check, for example an extra `DW_OP_uconst, 8, DW_OP_plus` just to ensure expression composition through your prependOffsetExpression implementation is covered in this test.

I'm assuming that the "*svint32_t" types act like aggregates in some way, and so can't have any other DWARF expression operators applied to them.


================
Comment at: llvm/test/CodeGen/AArch64/debug-info-sve-dbg-value.mir:41
+  attributes #0 = { "target-features"="+sve" }
+  attributes #1 = { nounwind readnone speculatable willreturn }
+
----------------
Could you remove the attributes for #1; for debug-info tests un-necessary attributes become an un-necessary burden.


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

https://reviews.llvm.org/D90020