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

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

Orlando planned changes to this revision.
Orlando added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:189
+      else
+        assert(false && "Failed to create fragment expr!");
+    }
----------------
jmorse wrote:
> `llvm_unreachable` is probably more cannonical. Or, assigning the Optional E, asserting it, then dereferencing it. Seems like un-necessary control flow otherwise.
I've removed the control flow, but I wonder if keeping both the `assert` and `if` is better for release mode in case there is a failure mode I haven't thought of here. I'm not 100% confident that this assert will never fire (in fact, I think it could) - I think it just hasn't fired for the code bases I've built. What do you think?


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:203
+
+    // We could use more precision here at the const of some additional (code)
+    // complexity - if the original dbg.assign was adjacent to its store, we
----------------
jmorse wrote:
> 
const incorrectness


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

https://reviews.llvm.org/D133296