[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:4714
@@ +4713,3 @@
+      // non-IEEE-754 compliant SIMD units.
+      } else if (it->getType()->isFloatingPointTy() &&
+                (it->isBinaryOp() || it->isCast()) &&
----------------
ashutosh.nema wrote:
> rengolin wrote:
> > ashutosh.nema wrote:
> > > You may want to set unsafe flag for some of the intrinsics as well.
> > > i.e. intrinsic like minnum, maxnum are floating point unsafe and 
> > > follows IEEE-754 semantics. 
> > > 
> > Hum, that's a good point.
> > 
> > But I don't want to be listing specific intrinsics if the list is not *very* short, or completely target independent.
> > 
> > Maybe I should just be safe and do that for all FP operations except move, load and store?
> Doing this for all operations except move, load and store would be conservative, this might miss eligible cases for vectorization.
> 
> If we list, then list should be for the safe intrinsic(those can be vectorized) and for others will mark ‘setPotentiallyUnsafe’.
> 
> Also need to handle function which are later going to be mapped to intrinsic (i.e. fmax, fmaxf).
> 
> llvm::getIntrinsicIDForCall returns intrinsic ID for vectorizable CallInst, in case it does not found it returns not_intrinsic.
> 
> Probably you can write a wrapper around this function and check safety.
> 
> llvm::isTriviallyVectorizable has a list of vectorizable intrinsics, might help during listing.
Can you guarantee that whatever list you put up will be final? I'd rather be conservative and expect the user to explicitly specify -ffast-math or -fsubnormal-math than get it wrong in the future.

There's no point in knowing which functions are vectoriseable if we don't which one of them can be affected by the "unsafe" behaviour on each target. On ARM NEON, it's subnormals, on others may be something else.

When we get to the point of creating the subnormal's flag, we'll have also to come up with a table for: is this instruction / call safe on this target? We don't need that for now, as we're assuming nothing is safe, just like GCC.

It *will* inhibit eligible cases, and that's intentional.


http://reviews.llvm.org/D18701