[PATCH] D147777: [Assignment Tracking] Fix fragments for assignments to variables smaller than the alloca

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.
This revision is now accepted and ready to land.

What would be the implications of a store that partially overlaps an AT tracked variable -- as a result of this patch, we would lose some of that information (to avoid a crash)? It feels like an extremely unlikely edge case, so doesn't worry me too much, but is the ultimate fix to truncate the assignment here, or just not produce that kind of debug-info in the first place? And if the latter, should we assert for that, or just conclude "SROA really hates what you're doing in this situation".

Otherwise LGTM



================
Comment at: llvm/lib/IR/DebugInfo.cpp:1849
+    // Discard stores to bits outside this variable. NOTE: trackAssignments
+    // doesn't understand base expressions yet, so all variables here are
+    // guarenteed to start at offset 0 in the alloca.
----------------
all variables that _reach_ here? Otherwise it's unclear what context "here" is in.


================
Comment at: llvm/lib/IR/DebugInfo.cpp:1850
+    // doesn't understand base expressions yet, so all variables here are
+    // guarenteed to start at offset 0 in the alloca.
+    if (Info.OffsetInBits + Info.SizeInBits > *Size)
----------------



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

https://reviews.llvm.org/D147777