[PATCH] D148018: [Assignment Tracking] Trunc fragments for stores to vars smaller than the alloca

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

Orlando added inline comments.


================
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.
----------------
jmorse wrote:
> Can we assert this, for when it likely changes in the future and we forget this code?
That info would come from the `VarRecord` which doesn't yet support any kind of offset. We could add a "dummy" field to that struct and then assert that it is zero here, but I'm not sure that's much better than the `VarStartBit = 0;` below. wdyt?


================
Comment at: llvm/lib/IR/DebugInfo.cpp:1860
+
+    FragStartBit = std::max(FragStartBit, VarStartBit);
+    FragEndBit = std::min(FragEndBit, VarEndBit);
----------------
jmorse wrote:
> Seeing how both inputs are unsigned and one argument is zero, isn't this guaranteed to always be `FragStartBit`? (Could produce a warning somewhere)
Yeah that's right, I was setting up for a future where it's not always zero. But good point about potential warning. I've replaced the line with a FIXME.


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

https://reviews.llvm.org/D148018