[PATCH] D18701: [ARM] Adding IEEE-754 SIMD detection to loop vectorizer

[Renato Golin via llvm-commits]

rengolin added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4727
@@ +4726,3 @@
+      } else if (it->getType()->isFloatingPointTy() &&
+                (CI || it->isBinaryOp() || it->isCast() || it->isShift()) &&
+                !it->hasUnsafeAlgebra()) {
----------------
hfinkel wrote:
> rengolin wrote:
> > hfinkel wrote:
> > > I don't understand why you have the `(CI || it->isBinaryOp() || it->isCast() || it->isShift())` - why does it matter what kind of operation it is? And do we have any floating-point shifts? If you specifically want to include casts, do you also specifically need to pick up loads and stores?
> > We don't want to get loads and stores because they're normally safe anyway. That's why I'm using the "else if".
> > 
> > But also I don't want to get MOVs and shuffles either, since there's no chance they'll change the semantics or precision unless it's specifically requested (widening/shortening, type change, etc). So I just selected everything else. :)
> > 
> > Function calls and binary operations are obvious. Casts may change the type, so they may end up as the case above. I don't know of any FP shifts, so that was probably over the top.
> > 
> > I think it should be safe to do:
> > 
> >     } else if (it->getType()->isFloatingPointTy() &&
> >                  (CI || it->isBinaryOp()) &&
> >                  !it->hasUnsafeAlgebra()) {
> > 
> That makes sense to me. We should add an associated note to the declaration of isFPVectorizationPotentiallyUnsafe indicating this. Something like this perhaps:
> 
>   // This applies to floating-point math operations and calls, not memory operations, shuffles, or casts.
> 
Will do, thanks!


http://reviews.llvm.org/D18701