[PATCH] D9151: Loop Versioning for LICM

[Nema, Ashutosh via llvm-commits]

Hal, is below comments on call handling looks OK to you ?


      >   Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:356

      >   @@ +355,3 @@

      >   +  const bool IsAnnotatedParallel = CurLoop->isAnnotatedParallel();

      >   +  // We dont support call instructions. however, we ignore few intrinsic

      >   +  // and libfunc callsite. We don't allow non-intrinsic, non-libfunc callsite

      >   ----------------

      >   Why not? I understand that the vectorizer has these checks, but I don't see why this belongs in an LICM pass?

      >

      > There is a possibility that call may modify aliasing behavior, which may defeat the purpose of versioning & runtime checks.


      Ah, good point. However, please then replace this check with an appropriate AA getModRef-type check for whether the call might alias with the relevant pointers (if the function, for example, does not alias with any of the loop-invariant accesses, then it can't affect what you're trying to do, although you'll need to be somewhat more careful about adding the noalias metadata to those functions, etc.).

I'm not sure that itself would be sufficient, consider indirect calls probably need to consider function pointer in runtime check.
Also there is a possibility that in call argument one of the runtime pointer escaped, in such cases it become more difficult to ensure correctness.
That's why for simplicity I considered vectorizer approach to consider only few functions.

Can I keep this like vectorizer ?

Regards,
Ashutosh


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151007/eb09552c/attachment.html>