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

[Hal Finkel via llvm-commits]

hfinkel 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()) {
----------------
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.



http://reviews.llvm.org/D18701