[PATCH] D21432: [ARM] Do not test for CPUs, use SubtargetFeatures (Part 1). NFC

[Renato Golin via llvm-commits]

rengolin added inline comments.

================
Comment at: lib/Target/ARM/ARM.td:137
@@ +136,3 @@
+// VFP to NEON.
+def FeatureNEONForFPMovs : SubtargetFeature<"neon-fpmovs", "UseNEONForFPMovs",
+                              "true", "Convert VMOVSR, VMOVRS, VMOVS to NEON">;
----------------
sbaranga wrote:
> They are both core-specific optimizations, so it makes sense that they are enabled selectively:
> 
> FeatureNEONForFPMoves enables changing the execution domain for mov-like instructions from VFP to NEON and is a core-specific optimization, so it makes sense to me that it's only enabled on Cortex-A9 (unless we have additional data points).
> 
> FeatureNEONforFP forces almost all floating point operations to be performed with NEON instructions (which is a problem if we care about how denormals are handled), so it makes sense to me that it's only enabled for swift/cyclone.
> 
> I think the distinction is that FeatureNEONForFP uses NEON for VFP movs only when it minimizes the domain crossing between NEON and VFP, while FeatureNEONforFP  does this everywhere (for movs and everything else). I'm not sure how you could improve the naming though.
So, just because one is a sub-set of the other, makes sense to have the same name + radical. But the extra behaviour (NEON/VFP barrier profitability) could be documented, at least in its comment.

About Swift's behaviour to ignore NEON subnormal problem, we have the same issue at the IR level (loop vectorizer), and I have "solved" the old way "ST->isTargetDarwin()", which is true at the platform level.

If you're still not happy with the naming, maybe, such a property could be called "FeatureIgnoreDenormalFP"?


http://reviews.llvm.org/D21432