[PATCH] D148018: [Assignment Tracking] Trunc fragments for stores to vars 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.

LGTM with some fairly standard nits



================
Comment at: llvm/lib/IR/DebugInfo.cpp:1854-1856
+    // NOTE: trackAssignments doesn't understand base expressions yet, so all
+    // variables that reach here are guaranteed to start at offset 0 in the
+    // alloca.
----------------
Can we assert this, for when it likely changes in the future and we forget this code?


================
Comment at: llvm/lib/IR/DebugInfo.cpp:1860
+
+    FragStartBit = std::max(FragStartBit, VarStartBit);
+    FragEndBit = std::min(FragEndBit, VarEndBit);
----------------
Seeing how both inputs are unsigned and one argument is zero, isn't this guaranteed to always be `FragStartBit`? (Could produce a warning somewhere)


================
Comment at: llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll:4-6
+;; sized stores to it, overlapping (or not) the variable in various ways. Check
+;; the fragment info looks right when converting the dbg.declare to
+;; dbg.assigns.
----------------
"looks right" is a bit too vague when someone has to update this test in the future, spelling it out a bit more would be better.


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

https://reviews.llvm.org/D148018