[PATCH] D21796: [ARM] Do not test for CPUs, use SubtargetFeatures (Part 3). NFCI

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 11:17:31 PDT 2016


MatzeB added inline comments.

================
Comment at: lib/Target/ARM/ARM.td:149-151
@@ +148,5 @@
+
+// Some targets use a compact unwind format which assumes that VFPs are
+// allocated with stride 4. This can expand the prologue, so it won't be taken
+// into account when optimizing for size.
+def FeatureUseStride4VFPs : SubtargetFeature<"use-stride4-vfps",
----------------
This explanation is odd, the unwind data format is an operating system detail not something CPU dependent.

================
Comment at: lib/Target/ARM/ARMSubtarget.h:398-400
@@ -384,2 +397,5 @@
 
+  /// @{
+  /// These functions are obsolete, please consider adding subtarget features
+  /// or properties instead of calling them.
   bool isCortexA5() const { return ARMProcFamily == CortexA5; }
----------------
Does this patch remove the last uses of those functions? If yes, then I would recommend removing these functions, otherwise it is just too easy to find them with grep or autocompletion while missing the deprecation comment.

Maybe go with the aarch64 solution of exposing a getProcFamily() function (that forces you to look around for the enum and write a little bit more which may be enough to get people reading the comment and thinking :)


http://reviews.llvm.org/D21796





More information about the llvm-commits mailing list