[PATCH] D143146: [Assignment Tracking] Fix migrateDebuginfo in SROA

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

Orlando added inline comments.


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2782
+    /// Return the index of the bit after the end of the fragment, e.g. for
+    /// fragment offset=0 size=32, return 32.
+    uint64_t endInBits() const { return OffsetInBits + SizeInBits; }
----------------
StephenTozer wrote:
> Nit so small you'd need an electron microscope (so feel free to ignore if you disagree), but imo if an illustrative example is being used to demonstrate that this returns the sum of the size and offset, they should both be non-zero values to make it clear that it's the sum and not just selecting the size.
This is reasonable and wise. Done.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:167-168
+  // target to fit.
+  if (Target.startInBits() < CurrentFragment->startInBits() ||
+      Target.endInBits() > CurrentFragment->endInBits())
+    return std::nullopt;
----------------
StephenTozer wrote:
> This check now subsumes the check above - since the fragment offset and size are always positive (we've got bigger problems if they aren't!), `CurrentFragment->endInBits() <= Target.startInBits()` will never be true if `Target.startInBits() < CurrentFragment->startInBits()` isn't (and same for the second condition).
This is true - removed the first check and updated the comments.


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

https://reviews.llvm.org/D143146