[PATCH] D69563: [LV] Strip wrap flags from vectorized reductions

[Denis Antrushin via Phabricator via llvm-commits]

dantrushin marked an inline comment as done.
dantrushin added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3706-3709
+    if (StripWrapFlags) {
+      cast<Instruction>(Val)->setHasNoUnsignedWrap(false);
+      cast<Instruction>(Val)->setHasNoSignedWrap(false);
+    }
----------------
lebedev.ri wrote:
> dantrushin wrote:
> > lebedev.ri wrote:
> > > ```
> > > if(auto* I = dyn_cast<Instruction>(LoopVal))
> > >   I->dropPoisonGeneratingFlags();
> > > ```
> > > Shouldn't this be done outside of this loop though?
> > No, I think I want them cleared  from every interleaved part
> > 
> 1. Still, 
> ```
> if(StripWrapFlags)
>   cast<Instruction>(Val)->dropPoisonGeneratingFlags();
> ```
> 2. Isn't the actual bug is in `getOrCreateVectorValue()`? why it returns such errneous instructions?
1.  But these flags are valid for Val -- it is (original) scalar instruction, which still present in original scalar loop, which serves as 
     reminder loop after vectorization. I want to preserve flags whereever they are valid, otherwise I just would not preserve them in
     InnerLoopVectorizer::widenInstruction:


```
4202       Value *V = Builder.CreateNAryOp(I.getOpcode(), Ops);
4203      
4204       if (auto *VecOp = dyn_cast<Instruction>(V))
4205         VecOp->copyIRFlags(&I);
```

2. `getOrCreateVectorValue()`  simply returns cached vector value from `VectorLoopValueMap`.
   And this instruction was created by `InnerLoopVectorized::widenInstruction()`, which lacks necessary context.
   So we basically back to the original question - if we do not want to preserve valid flags at all, we can simply not copy them in 
   `widenInstruction()` (VectorOp->copyIRFlags(&I, **false**) ).
   But if we do want to preserve flags where they're valid, we need a context and clear them only for reductions. 

  




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69563