[PATCH] D111846: [LV] Drop NUW/NSW flags from instructions that need predication

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3045
+// linearizing the control flow as a result of vectorization.
+static void clearUnsafeFlagsAfterLinearizingCF(Instruction *NewInst) {
+  if (isa<OverflowingBinaryOperator>(NewInst)) {
----------------
dcaballe wrote:
> spatel wrote:
> > This could be Instruction::dropPoisonGeneratingFlags() instead of new code? That also handles GEP's `inbounds` flag. If `inbounds` is also a concern with this transform, can we add a test for it?
> > 
> > Note that `dropPoisonGeneratingFlags` has a TODO comment for FMF.
> > That also handles GEP's inbounds flag. If inbounds is also a concern with this transform, can we add a test for it?
> 
> Thanks! That makes sense, I think. The load/store will be inbounds because we are masking out out-of-bounds elements but the GEP could be out-of-bounds.
> 
> > This could be Instruction::dropPoisonGeneratingFlags() instead of new code? 
> 
> Sure. The only problem I see is extending this utility to FMF. The utility is broadly used in LLVM (ValueTracking, LoopUtils, SimplifyIndVar, InstCombine*, BDCE, etc.). It might be hard to add FMF if that breaks all these passes, but we can try.
I think it's low risk - those passes are mostly dealing with integer transforms when using that call.
Even if we change something by including FMF (nnan and ninf) in the list, it should be a perf regression at worst (we shouldn't break correctness by removing FMF or wrap/exact flags).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111846