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

[Hal Finkel via llvm-commits]

hfinkel added a comment.

Yes, this is essentially what I had in mind. Two comments:


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:372
@@ +371,3 @@
+  /// (like sub-normals in NEON v7) via unsafe math flags.
+  bool enablePotentiallyUnsafeFPVectorization() const;
+
----------------
The name of this method, and the corresponding comment, seem potentially confusing (no pun intended ;) ). It does not explain which floating-point operations are potentially unsafe (essentially all of them?). I think it would be much better to name this:

  bool isFPVectorizationPotentiallyUnsafe() const;

perhaps with this comment:

  /// \brief Indicate that it is potentially unsafe to automatically vectorize floating-point operations because the semantics of vector and scalar floating-point semantics may differ. For example, ARM NEON v7 SIMD math does not support IEEE-754 denormal numbers, while depending on the platform, scalar floating-point math does.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4727
@@ +4726,3 @@
+      } else if (it->getType()->isFloatingPointTy() &&
+                (CI || it->isBinaryOp() || it->isCast() || it->isShift()) &&
+                !it->hasUnsafeAlgebra()) {
----------------
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?


http://reviews.llvm.org/D18701