[PATCH] D138992: [DebugInfo][SROA] Correct debug info for global variables spanning multiple fragments in case of SROA

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok marked an inline comment as done.
alok added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:432
     uint64_t CurVarSize = Var->getType()->getSizeInBits();
+    CurVarEndInBits = CurVarOffsetInBits + CurVarSize;
     // Current variable ends before start of fragment, ignore.
----------------
aprantl wrote:
> Why initialize to zero and then again here?
> Why initialize to zero and then again here?

Thanks for pointing this out. I have now removed initialize with zero.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:438
 
-    // Current variable fits in the fragment.
-    if (CurVarOffsetInBits == FragmentOffsetInBits &&
-        CurVarSize == FragmentSizeInBits)
-      Expr = DIExpression::get(Expr->getContext(), {});
-    // If the FragmentSize is smaller than the variable,
-    // emit a fragment expression.
-    else if (FragmentSizeInBits < VarSize) {
+    // If the FragmentSize is greater than or equal to the current variable.
+    if (CurVarSize != 0 && /* CurVarSize is known */
----------------
aprantl wrote:
> tbh, I found the original comment easier to understand
> tbh, I found the original comment easier to understand

Thanks, the comment is updated now.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:440
+    if (CurVarSize != 0 && /* CurVarSize is known */
+        CurVarOffsetInBits >= FragmentOffsetInBits &&
+        CurVarEndInBits <= FragmentEndInBits) {
----------------
aprantl wrote:
> Aren't these two conditions trivially true because of the check above?
> Aren't these two conditions trivially true because of the check above?

Checks at line 427 and 433 assure that current variable does not overlap with current fragment.
Current condition further checks whether current variable overlaps only (fits) current fragment, failing check is when current variable overlaps other fragments also and needs fragment expression.


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

https://reviews.llvm.org/D138992