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

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:414
+        Expr->getElement(0) == dwarf::DW_OP_plus_uconst) {
+      CurVarOffset = 8 * Expr->getElement(1);
+    }
----------------
alok wrote:
> aprantl wrote:
> > This hardcoded 8 looks suspicious. What is it derived from and why is it a constant?
> This is byte to bit conversion.
the CHAR_BIT is ok here


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:423
+    // Current variable ends before start of fragment, ignore.
+    if ((CurVarSize != 0) &&
+        ((CurVarOffset + CurVarSize) <= FragmentOffsetInBits))
----------------
we have some extra parentheses here


================
Comment at: llvm/test/DebugInfo/Generic/global-sra-struct-fit-segment.ll:99-101
+declare i32 @__atomic_compare_exchange(i64, i8* noalias, i8* noalias, i8* noalias, i32, i32)
+
+declare i64 @__tgt_fort_ptr_assn_i8(i8* noalias, i8* noalias, i8* noalias, i8* noalias, i8* noalias)
----------------
unused, so it can be deleted


================
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())
+
----------------
Orlando wrote:
> Please can you replace the hardcoded metadata (`!1`, `!8`, `!22`) in these check lines?
+1


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

https://reviews.llvm.org/D121107