[PATCH] D133296: [Assignment Tracking][13/*] Account for assignment tracking in SROA

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

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


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:3051
+        if (isa<Constant>(II.getLength())) {
+          assert((NewBeginOffset - NewAllocaBeginOffset == 0 ||
+                  at::getAssignmentMarkers(&II).empty()) &&
----------------
Orlando wrote:
> jmorse wrote:
> > First option simplifiable to `NewBeginOffset == NewAllocaBeginOffset`? It's not immediately obvious what this is checking, and I feel it might be easier to read if we name the other proposition "NoAssignmentMarkers" or something. That might also allow you to condense or invert the control flow. The for loop is dependent on !NoAssignmentMarkers, for example.
> I think this blob represents me not being sure what to do here at the time. I don't think the for loop below is correct - based on other similar changes in the patch stack that probably needs to be a replaceUsesOfWith style update. It turns out I didn't have test coverage for this. sitrep: I've now got a test that stimulates this codepath (not uploaded) - but not yet managed to get one that let's us inspect the state at this point (it goes through another round of SROA-ing).
Still working on this test - no luck so far.


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

https://reviews.llvm.org/D133296