[llvm] [SROA] Fix incorrect offsets for structured binding variables (PR #69007)

[Orlando Cazalet-Hyams via llvm-commits]

================
@@ -1782,8 +1782,34 @@ void at::deleteAll(Function *F) {
 
 bool at::calculateFragmentIntersect(
     const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
-    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DAI,
-    std::optional<DIExpression::FragmentInfo> &Result) {
+    uint64_t SliceSizeInBits, const DbgVariableIntrinsic *DVI,
+    std::optional<DIExpression::FragmentInfo> &Result,
+    uint64_t &NewExprOffsetInBits) {
+
+  // Only dbg.assign and dbg.declares are allowed because this function
+  // deals with memory locations. This isn't comprehensive because dbg.values
+  // are able to describe memory locations; support for dbg.values can be added
+  // if/when needed.
+  assert(isa<DbgAssignIntrinsic>(DVI) || isa<DbgDeclareInst>(DVI));
+
+  // There isn't a shared interface to get the "address" parts out of a
+  // dbg.declare and dbg.assign, so provide some wrappers now.
----------------
OCHyams wrote:

I'm not 100% sure, I was on the fence about it so I erred on the side of minimising changes.

What would you suggest we do for the `DbgValueInst` overloads? I suppose that we could just return nullptr from `getAddress` (as there's no "address" part of a dbg.value).

I think it could get a little confusing for some of them, e.g. if we add `getAddressExpression`. `getExpression` != `getAddressExpression` for dbg.assign, but they are equal for a dbg.declare, and for a dbg.value `getAddressExpression` would either return nullptr or possibly confusingly also be an alias for `getExpression`. Then again, sufficient doc-comments should be able to explain the differences.

(I'm going to return to this patch a bit later, but thought I should get your thoughts on this while it's relatively fresh)

https://github.com/llvm/llvm-project/pull/69007