[PATCH] [ARM] Add knowledge of FPU subtarget features to TargetParser

Renato Golin renato.golin at linaro.org
Thu Jun 4 05:49:06 PDT 2015


Hi John,

Thanks for working on this.

I think this patch could be split into smaller chunks, and move other chunks elsewhere.

One patch is to add the enums, update the table and create the get methods. No <vector>.

Another is to move the parsing logic from ARMAsmParser to a new method, one outside the ARMTargetParser and ARMAsmParser, that uses a Features vector and common both assembler's and Clang's feature bits parsing, but only changes in the assembler side (LLVM patch). ARMSubTarget is probably the best place for it.

The final patch (http://reviews.llvm.org/D10239), will then just move to this new method.

Makes sense?

cheers,
--renato


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Support/TargetParser.h:18
@@ -17,1 +17,3 @@
 
+#include <vector>
+
----------------
This will include <vector> is a lot of other files that don't need it. Can you use SmallVector or something and declare it like StringRef?

================
Comment at: lib/Support/TargetParser.cpp:27
@@ +26,3 @@
+// neon with crypto.
+enum NeonSupportLevel {
+  NS_None,
----------------
I'm leaving enums in the header file for now. It may not be pretty to expose the internal of it for now, but we need to understand where things are used and why, so that when we implement the new Tuple class (see the list for more details), we can merge enums from Triple, TargetParser and others into one big, internal, implementation detail.

================
Comment at: lib/Support/TargetParser.cpp:253
@@ -228,1 +252,3 @@
 
+bool ARMTargetParser::getFPUFeatures(unsigned FPUKind, std::vector<const char *> &Features) {
+
----------------
I think this method is too specific, and it should be implemented in the user's side.

The overall design of this class must be as simple as possible, even if that means more code on the user side. So, create two new methods getRestriction() and getNeonSupport() by using the two new columns provided and implement the rest in the caller's side.


================
Comment at: lib/Support/TargetParser.cpp:257
@@ +256,3 @@
+    return false;
+  }
+
----------------
No need for brackets here.

http://reviews.llvm.org/D10237

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list