jojo added inline comments.
================
Comment at: include/llvm/Support/AArch64TargetParser.def:20
@@ +19,3 @@
+AARCH64_ARCH("armv8-a", AK_ARMV8A, "8-A", "v8", ARMBuildAttrs::CPUArch::v8_A,
+ FK_CRYPTO_NEON_FP_ARMV8, (AArch64::AEK_CRC | AArch64::AEK_CRYPTO |
AArch64::AEK_FP | AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_PROFILE))
+AARCH64_ARCH("armv8.1-a", AK_ARMV8_1A, "8.1-A", "v8.1a",
ARMBuildAttrs::CPUArch::v8_A,
----------------
rengolin wrote:
> Please, format this file to 80-columns.
I neglected this.Format it in the update.
================
Comment at: include/llvm/Support/AArch64TargetParser.def:31
@@ +30,3 @@
+// FIXME: This would be nicer were it tablegen
+AARCH64_ARCH_EXT_NAME("invalid", AArch64::AEK_INVALID, nullptr, nullptr)
+AARCH64_ARCH_EXT_NAME("none", AArch64::AEK_NONE, nullptr, nullptr)
----------------
rengolin wrote:
> Same comment as the ARM def file, wrapping with "namespace AArch64 { }".
Looks like the namespace is not supported in the def file. I tried this in the
early time.
================
Comment at: include/llvm/Support/ARMTargetParser.def:48
@@ -47,3 +47,3 @@
ARM_ARCH("invalid", AK_INVALID, nullptr, nullptr,
- ARMBuildAttrs::CPUArch::Pre_v4, FK_NONE, AEK_NONE)
+ ARMBuildAttrs::CPUArch::Pre_v4, FK_NONE, ARM::AEK_NONE)
ARM_ARCH("armv2", AK_ARMV2, "2", "v2", ARMBuildAttrs::CPUArch::Pre_v4,
----------------
rengolin wrote:
> This is the "ARM" def file, maybe wrap with "namespace ARM { }"?
Same as "AArch64" def file.
================
Comment at: include/llvm/Support/TargetParser.h:144
@@ +143,3 @@
+
+namespace AArch64 {
+
----------------
rengolin wrote:
> Please, add a FIXME line here, saying this really should be made into a
> class, to use ARM's methods from inheritance.
Well,yes.Done it in the update.
================
Comment at: lib/Support/TargetParser.cpp:764
@@ +763,3 @@
+ Arch = getCanonicalArchName(Arch);
+ if (!Arch.startswith("v8"))
+ return ARM::AK_INVALID;
----------------
rengolin wrote:
> Maybe a better check here would be:
>
> if (checkArchVersion(Arch) < 8)
> return ARM::AK_INVALID;
Good advice.Put the check in a method alone.
================
Comment at: lib/Support/TargetParser.cpp:770
@@ +769,3 @@
+ if (A.getName().endswith(Syn)) {
+ if (A.ID == ARM::AK_ARMV8_1A)
+ Features.push_back("+v8.1a");
----------------
rengolin wrote:
> I don't like mixing Features with parseArch. Maybe we could do two different
> methods and get callers to use both when needed, just like with ARM.
>
> parseArch needs to be dead simple.
Yes.Moving this to another method "getArchFeatures".
http://reviews.llvm.org/D20089
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
