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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked 2 inline comments as done.
sdesmalen added a comment.

Hi @jmorse, thanks for reviewing this patch!

In D90020#2436998 <https://reviews.llvm.org/D90020#2436998>, @jmorse wrote:

> (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.

Yes, I think that's a good summary. It is indeed a way to describe a runtime constant, rather than describing a register, since there is no physical register associated with it.

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

Thanks, will do!



================
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
+#
----------------
jmorse wrote:
> 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.
> 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.
Good suggestion, I've updated the 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.
I think that's right, svint32_t are more or less opaque vector types. Do you have an example of expressions that would apply for regular (fixed-width) vectors?


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

https://reviews.llvm.org/D90020