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

[David Stenberg via Phabricator via llvm-commits]

dstenb added a comment.

I'm sorry for chiming in so late here! I have a comment about the `prependOffsetExpression` target hook.



================
Comment at: llvm/lib/CodeGen/TargetRegisterInfo.cpp:538
+TargetRegisterInfo::prependOffsetExpression(const DIExpression *Expr,
+                                            unsigned PrependFlags,
+                                            const StackOffset &Offset) const {
----------------
I think I would be better to have explicit `bool` arguments for each of the flags that are contained here. If we keep the mask here, we could later on, when introducing a new flag, end up in situations where the new flag would be ignored by some target implementations, and that could be hard to detect.

Another solution could be to have the deref, stack value and entry value handling in this function, and then have it invoke a `getOffsetOpcodes(Offset, Ops)` target hook, or something along those lines, that produces the target specific expression for the offset.

At the risk of me overlooking some details for why it would be feasible or practical, I think I would prefer the latter interface.


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

https://reviews.llvm.org/D90020