bsmith added a comment.

There is an awful lot of duplication/passing through to another class in this, 
it strikes me that this whole thing could benefit from some level of 
inheritance. I think it would be good to have a base class that defines the 
interface and have both ARM/AArch64 (and any other architectures that want to 
use this in the future) implement this interface. That way all of this code can 
be called generically from clang/wherever.


================
Comment at: lib/Support/TargetParser.cpp:441
@@ +440,3 @@
+  if (Extensions & AArch64::AEK_PROFILE)
+    Features.push_back("+spe");
+
----------------
For ARM there is a table that defines these extensions and how they map to 
backend features, it would be good to do this in a similar manner.

================
Comment at: lib/Support/TargetParser.cpp:471
@@ +470,3 @@
+  if (ArchKind >= ARM::AK_LAST)
+    return ARMBuildAttrs::CPUArch::Pre_v4;
+  return AArch64ARCHNames[ArchKind].ArchAttr;
----------------
This doesn't make sense for AArch64

================
Comment at: lib/Support/TargetParser.cpp:770
@@ +769,3 @@
+      if (A.ID == ARM::AK_ARMV8_2A)
+        Features.push_back("+v8.2a");
+      return A.ID;
----------------
Why do we need to add these features explicitly, can't we just pass through the 
correct triple?


Repository:
  rL LLVM

http://reviews.llvm.org/D20089



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to