[PATCH] D121107: [DebugInfo][SROA] Correct debug info for global variables in case of SROA

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Please wait for the other reviewers to be happy, but this LGTM with some nits addressed.



================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:439
       else
         return;
     }
----------------
This is unrelated to your change but I am curious - sorry for the noise and possibly silly question. Should this `return` be a `continue`? cc @aprantl.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:412-413
+    // Calculate the offset (Bytes), Convert to bits.
+    if (Expr->getNumElements() == 2 &&
+        Expr->getElement(0) == dwarf::DW_OP_plus_uconst) {
+      CurVarOffset = CHAR_BIT * Expr->getElement(1);
----------------
I think it is worth using `DIExpression::extractIfOffset` to get the offset here instead of inspecting the expression manually.


================
Comment at: llvm/test/DebugInfo/Generic/global-sra-struct-fit-segment.ll:1
+; REQUIRES: x86_64-linux
+
----------------
Can you move this to `llvm/test/DebugInfo/X86`? The tests there only run if `X86` is a registered target.


================
Comment at: llvm/test/DebugInfo/Generic/global-sra-struct-fit-segment.ll:108
+; CHECK-DAG: ![[GVE2]] = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+; CHECK-DAG: ![[GVE3]] = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
+
----------------
Please can you replace the hardcoded metadata (`!1`, `!8`, `!22`) in these check lines?


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

https://reviews.llvm.org/D121107