[PATCH] D32674: [LoopIdiom] PR32811 check for safety while expanding

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:823
   Value *NumBytes =
       Expander.expandCodeFor(NumBytesS, IntPtr, Preheader->getTerminator());
 
----------------
efriedma wrote:
> hiraditya wrote:
> > efriedma wrote:
> > > Do we also need to check whether it's safe to expand NumBytesS?
> > I don't think so (IIUC), because the NumBytes is based on the trip count of the loop*storesize, both of them should be safe to expand if the loop has a valid SCEV.
> The store size is a constant, so that's obviously fine, but I don't see how the loop's trip count is obviously safe to expand; .  Consider, e.g.:
> 
> ```
> #include <stdio.h>
> void f(unsigned a, unsigned b, char* x) {
>   for (int j = 0; j < a; j++)
>     if (b != 0)
>       for (long long i = 0; i < a / b; ++i)
>         x[i] = 0;
> }
> ```
Thanks for the test case. You are right, this test does hoist udiv even with this patch. I'll update the patch and add this test case.


https://reviews.llvm.org/D32674