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

[silviu.baranga@arm.com via llvm-commits]

sbaranga 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">;
----------------
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.

================
Comment at: lib/Target/ARM/ARMInstrInfo.td:329
@@ +328,3 @@
+
+def UseVMOVSR : Predicate<"Subtarget->preferVMOVSR() ||"
+                          "!Subtarget->useNEONForSinglePrecisionFP()">;
----------------
Is there any reason not to test for useNEONForFPMovs instead?


http://reviews.llvm.org/D21432