[PATCH] D46750: [SROA] pr37267: fix assertion failure while integer widening

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs added a comment.

> Your testcase doesn't actually crash on trunk, unlike the testcase from the bug.

I fixed the test case. Thank you for pointing this out.



================
Comment at: lib/Transforms/Scalar/SROA.cpp:1994
+    // We can't handle split slice tails.
+    if (RelBegin < 0)
+      return false;
----------------
efriedma wrote:
> inouehrs wrote:
> > efriedma wrote:
> > > Why does this check belong here, as opposed to the beginning of the function or somewhere in isIntegerWideningViable()?
> > `visitMemTransferInst` seems to be able to handle split tails of memcpy (e.g. when replacing a store to slice 1 in the test case with memcpy). So I do not want to reduce the opportunities by checking this at the beginning of `isIntegerWideningViableForSlice`.
> So the limitation isn't that the transform is impossible, just that rewriteIntegerStore doesn't know how to insert the right shift?  If that's correct, could you state that in the comment a little more explicitly?  If not, could you explain a bit more why this isn't possible?
> 
> The testcase from the bug specifically hits the store case; can you construct a testcase which hits the load case?
I added the second test case which causes an assertion failure while rewriting a load.


https://reviews.llvm.org/D46750