[PATCH] D36244: [LoopVectorize] Fix assertion failure in Fcmp vectorization

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4867
+        IRBuilder<>::FastMathFlagGuard FMFG(Builder);
+        Builder.setFastMathFlags(Cmp->getFastMathFlags());
         C = Builder.CreateFCmp(Cmp->getPredicate(), A, B);
----------------
Ayal wrote:
> An alternative may be to do
> 
> ```
>   if (isa<FCmpInst>(C))
>     cast<FCmpInst>(C)->copyFastMathFlags(Cmp);
> ```
> or
> 
> ```
>   if (auto *FCmp = dyn_cast<FCmpInst>(C))
>     FCmp->copyFastMathFlags(Cmp);
> ```
> 
> 
> Wonder if the last, optional argument to CreateFCmp() may be used instead?
> 
> Is this issue relevant elsewhere, e.g., SLP vectorizer? (Maybe that last argument shouldn't be optional ;-)
the alternative way may also be used. 

The `FastMathFlagGuard` seems to be the accepted practice though - in all of instcombine and LoopUtils transformations where the created instruction may not be a compare. 

`SLPVectorizer` creates FCmp and uses `propagateIRFlags` from LoopUtils for this. So, SLPVectorizer does not have this bug. However, it's a full blown method that copies IR flags by checking the type of instruction, and we don't need it here in LoopVectorizer. 


https://reviews.llvm.org/D36244