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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:1143
+      Ops.push_back(-ScaledOffset);
+      Ops.append({dwarf::DW_OP_bregx, *ScaleReg, 0ULL});
+      Ops.push_back(dwarf::DW_OP_mul);
----------------
aprantl wrote:
> We currently don't allow hardcoded registers inside of a DIExpression, The clean solution for this would be to rebase this on the patch that adds support for DW_OP_LLVM_argX / DBG_VALUE_LIST.
Hi @aprantl, thanks for having a look at this patch.

I looked at the proposal for DW_OP_LLVM_argX/DBG_VALUE_LIST and it seems that the scope of that work is a lot wider than what's needed for this purpose and I don't immediately see how I would represent these expressions using that mechanism, specifically because ScaleReg itself has no value in IR/MIR.

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. Hence the reason that [[ https://github.com/ARM-software/abi-aa/blob/master/aadwarf64/aadwarf64.rst | DWARF for the Arm® 64-bit Architecture (AArch64) ]] specifies the VG register to contain this value, so it can be used in forming DWARF expressions, e.g. to say that an offset (or number of elements when describing a vector type) is scaled by VG.

I can see why it is not customary or considered bad practice to hard-code registers directly in a DIExpression, but in this case I don't think this is really this a problem, given that in this context the register should actually be a hardcoded register.

Given this somewhat limited use-case, I'm not sure if the concern here is with the interface (i.e. we don't want to expose an explicit `ScaleReg` in this interface, as that opens the interface up to wrong uses of it), or whether the problem really is with the data that the DIExpression contains. i.e. Is there anything fundamental that prevents a DIExpression from containing a hard-coded register?

If not, I could modify the patch to have the target return a self-built DIExpression for the given offset, that can be prepended in PEI. If that would be fine, then I would much prefer that route over rebasing on what seems like a significant functional refactoring effort, which may take a while before it all lands.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90020