Re: [PATCH] D23293: Some place that could using TargetParser in clang
jojo closed this revision. jojo added a comment. Committed r278890. https://reviews.llvm.org/D23293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23293: Some place that could using TargetParser in clang
jojo created this revision.
jojo added a reviewer: rengolin.
jojo added a subscriber: cfe-commits.
Some missing usage of TargetParser in Tools.cpp and Targets.cpp.
https://reviews.llvm.org/D23293
Files:
lib/Basic/Targets.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -7458,11 +7458,14 @@
void darwin::setTripleTypeForMachOArchName(llvm::Triple &T, StringRef Str) {
const llvm::Triple::ArchType Arch = getArchTypeForMachOArchName(Str);
+ unsigned ArchKind = llvm::ARM::parseArch(Str);
T.setArch(Arch);
if (Str == "x86_64h")
T.setArchName(Str);
- else if (Str == "armv6m" || Str == "armv7m" || Str == "armv7em") {
+ else if (ArchKind == llvm::ARM::AK_ARMV6M ||
+ ArchKind == llvm::ARM::AK_ARMV7M ||
+ ArchKind == llvm::ARM::AK_ARMV7EM) {
T.setOS(llvm::Triple::UnknownOS);
T.setObjectFormat(llvm::Triple::MachO);
}
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -4874,7 +4874,7 @@
// the frontend matches that.
if (Triple.getEnvironment() == llvm::Triple::EABI ||
Triple.getOS() == llvm::Triple::UnknownOS ||
- StringRef(CPU).startswith("cortex-m")) {
+ ArchProfile == llvm::ARM::PK_M) {
setABI("aapcs");
} else if (Triple.isWatchABI()) {
setABI("aapcs16");
@@ -5203,7 +5203,7 @@
if (SoftFloat)
Builder.defineMacro("__SOFTFP__");
-if (CPU == "xscale")
+if (ArchKind == llvm::ARM::AK_XSCALE)
Builder.defineMacro("__XSCALE__");
if (isThumb()) {
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -7458,11 +7458,14 @@
void darwin::setTripleTypeForMachOArchName(llvm::Triple &T, StringRef Str) {
const llvm::Triple::ArchType Arch = getArchTypeForMachOArchName(Str);
+ unsigned ArchKind = llvm::ARM::parseArch(Str);
T.setArch(Arch);
if (Str == "x86_64h")
T.setArchName(Str);
- else if (Str == "armv6m" || Str == "armv7m" || Str == "armv7em") {
+ else if (ArchKind == llvm::ARM::AK_ARMV6M ||
+ ArchKind == llvm::ARM::AK_ARMV7M ||
+ ArchKind == llvm::ARM::AK_ARMV7EM) {
T.setOS(llvm::Triple::UnknownOS);
T.setObjectFormat(llvm::Triple::MachO);
}
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -4874,7 +4874,7 @@
// the frontend matches that.
if (Triple.getEnvironment() == llvm::Triple::EABI ||
Triple.getOS() == llvm::Triple::UnknownOS ||
- StringRef(CPU).startswith("cortex-m")) {
+ ArchProfile == llvm::ARM::PK_M) {
setABI("aapcs");
} else if (Triple.isWatchABI()) {
setABI("aapcs16");
@@ -5203,7 +5203,7 @@
if (SoftFloat)
Builder.defineMacro("__SOFTFP__");
-if (CPU == "xscale")
+if (ArchKind == llvm::ARM::AK_XSCALE)
Builder.defineMacro("__XSCALE__");
if (isThumb()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20089: Adding a TargetParser for AArch64
jojo created this revision.
jojo added reviewers: bsmith, jmolloy, rengolin.
jojo added subscribers: llvm-commits, cfe-commits.
jojo set the repository for this revision to rL LLVM.
jojo changed the visibility of this Differential Revision from "Public (No
Login Required)" to "All Users".
Herald added subscribers: rengolin, aemerson.
Adding a TargetParser for AArch64
Repository:
rL LLVM
http://reviews.llvm.org/D20089
Files:
include/llvm/Support/ARMTargetParser.def
include/llvm/Support/TargetParser.h
lib/Support/TargetParser.cpp
Index: lib/Support/TargetParser.cpp
===
--- lib/Support/TargetParser.cpp
+++ lib/Support/TargetParser.cpp
@@ -21,6 +21,7 @@
using namespace llvm;
using namespace ARM;
+using namespace AArch64;
namespace {
@@ -75,6 +76,11 @@
{NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHNames[] = {
+#define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT) \
+ {NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
+ sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of Arch Extension names.
@@ -91,6 +97,10 @@
#define ARM_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
{ NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHExtNames[] = {
+#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
+ { NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of HWDiv names (use getHWDivSynonym) and which architectural
@@ -124,6 +134,10 @@
#define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
{ NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64CPUNames[] = {
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+ { NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
+#include "llvm/Support/AArch64TargetParser.def"
};
} // namespace
@@ -369,6 +383,134 @@
return "generic";
}
+StringRef llvm::AArch64::getFPUName(unsigned FPUKind) {
+ return ARM::getFPUName(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUVersion(unsigned FPUKind) {
+ return ARM::getFPUVersion(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUNeonSupportLevel(unsigned FPUKind) {
+ return ARM::getFPUNeonSupportLevel( FPUKind);
+}
+
+unsigned llvm::AArch64::getFPURestriction(unsigned FPUKind) {
+ return ARM::getFPURestriction(FPUKind);
+}
+
+unsigned llvm::AArch64::getDefaultFPU(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].DefaultFPU;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, DEFAULT_FPU)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(ARM::FK_INVALID);
+}
+
+unsigned llvm::AArch64::getDefaultExtensions(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].ArchBaseExtensions;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, AArch64ARCHNames[ID].ArchBaseExtensions | DEFAULT_EXT)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(AArch64::AEK_INVALID);
+}
+
+bool llvm::AArch64::getExtensionFeatures(unsigned Extensions,
+ std::vector &Features) {
+
+ if (Extensions == AArch64::AEK_INVALID)
+return false;
+
+ if (Extensions & AArch64::AEK_FP)
+Features.push_back("+fp-armv8");
+ if (Extensions & AArch64::AEK_SIMD)
+Features.push_back("+neon");
+ if (Extensions & AArch64::AEK_CRC)
+Features.push_back("+crc");
+ if (Extensions & AArch64::AEK_CRYPTO)
+Features.push_back("+crypto");
+ if (Extensions & AArch64::AEK_FP16)
+Features.push_back("+fullfp16");
+ if (Extensions & AArch64::AEK_PROFILE)
+Features.push_back("+spe");
+
+ return true;
+}
+
+bool llvm::AArch64::getFPUFeatures(unsigned FPUKind,
+ std::vector &Features) {
+ return ARM::getFPUFeatures(FPUKind, Features);
+}
+
+StringRef llvm::AArch64::getArchName(unsigned ArchKind) {
+ if (ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getName();
+}
+
+StringRef llvm::AArch64::getCPUAttr(unsigned ArchKind) {
+ if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getCPUAttr();
+}
+
+StringRef llvm::AArch64::getSubArch(unsigned ArchKind) {
+ if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getSubArch();
+}
+
+unsigned llvm::AArch64::getA
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo added a comment. Dear Bradley,Renato Sorry for late reply,I have been on leave the last three days. Thank you very much for your review and suggestons.I will re pondering the changes. 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
Re: [PATCH] D20088: Using AArch64TargetParser in clang
jojo added a comment. Dear Bradley,Renato Sorry for late reply,I have been on leave the last three days. Thank you very much for your review and suggestons.I will re pondering the changes. Repository: rL LLVM http://reviews.llvm.org/D20088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo added inline comments.
Comment at: lib/Support/TargetParser.cpp:441
@@ +440,3 @@
+ if (Extensions & AArch64::AEK_PROFILE)
+Features.push_back("+spe");
+
bsmith wrote:
> 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.
There is a similar one for aarch64.But only in the context of switch-case,they
can be map to backend features by table look-up. eg,getArchExtName().
We need to judge the value of "Extensions" by means of bit operations here.It
is not possible to use the table.
Comment at: lib/Support/TargetParser.cpp:770
@@ +769,3 @@
+ if (A.ID == ARM::AK_ARMV8_2A)
+Features.push_back("+v8.2a");
+ return A.ID;
bsmith wrote:
> Why do we need to add these features explicitly, can't we just pass through
> the correct triple?
Sometimes people do not give a complete and standardized triple, which leads to
the obtaining of these features by triple impossible.
It maybe safer to add these features by "-march".
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
Re: [PATCH] D20088: Using AArch64TargetParser in clang
jojo added inline comments.
Comment at: lib/Driver/Tools.cpp:707
@@ -696,3 +706,3 @@
std::string MArch = arm::getARMArch(ArchName, Triple);
- if (llvm::ARM::parseArch(MArch) == llvm::ARM::AK_INVALID ||
+ if (!checkARMArchValid(MArch) || llvm::ARM::parseArch(MArch) ==
llvm::ARM::AK_INVALID ||
(Split.second.size() && !DecodeARMFeatures(D, Split.second, Features)))
bsmith wrote:
> Why do we need the call to checkARMArchValid here? Isn't is sufficient that
> parseArch returns a valid architecture?
Let the TargetParser return correct result.Remove this in the next patchset.
Comment at: lib/Driver/Tools.cpp:2280
@@ -2276,12 +2279,3 @@
- if (Split.first == "armv8-a" || Split.first == "armv8a") {
-// ok, no additional features.
- } else if (Split.first == "armv8.1-a" || Split.first == "armv8.1a") {
-Features.push_back("+v8.1a");
- } else if (Split.first == "armv8.2-a" || Split.first == "armv8.2a" ) {
-Features.push_back("+v8.2a");
- } else {
-return false;
- }
-
- if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
+ if (!checkAArch64ArchValid(Split.first) ||
llvm::AArch64::parseArch(Split.first, Features) == llvm::ARM::AK_INVALID ||
+ (Split.second.size() && !DecodeAArch64Features(D, Split.second,
Features)))
bsmith wrote:
> Same here, why do we need checkAArch64ArchValid?
Same as above.
Repository:
rL LLVM
http://reviews.llvm.org/D20088
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo 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. Dear Bradley, Thank you very much for such good advice. Making the TargetParser into class-base design will need large changes.I think it maybe better to do that in a steady way. So we can do it step by step.Just make a similar one for aarch64 at first,then make both of them into class. what's your opinion? 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
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo removed rL LLVM as the repository for this revision.
jojo changed the visibility of this Differential Revision from "All Users" to
"Public (No Login Required)".
jojo updated this revision to Diff 58071.
jojo added a comment.
1.unsigned llvm::AArch64::getArchAttr(unsigned ArchKind)
Correct the error according to the inline comment.
2.unsigned llvm::AArch64::parseArch(StringRef Arch, std::vector
&Features)
Adjust arch check logic
http://reviews.llvm.org/D20089
Files:
include/llvm/Support/AArch64TargetParser.def
include/llvm/Support/ARMTargetParser.def
include/llvm/Support/TargetParser.h
lib/Support/TargetParser.cpp
Index: lib/Support/TargetParser.cpp
===
--- lib/Support/TargetParser.cpp
+++ lib/Support/TargetParser.cpp
@@ -21,6 +21,7 @@
using namespace llvm;
using namespace ARM;
+using namespace AArch64;
namespace {
@@ -75,6 +76,11 @@
{NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHNames[] = {
+#define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT) \
+ {NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
+ sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of Arch Extension names.
@@ -91,6 +97,10 @@
#define ARM_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
{ NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHExtNames[] = {
+#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
+ { NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of HWDiv names (use getHWDivSynonym) and which architectural
@@ -124,6 +134,10 @@
#define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
{ NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64CPUNames[] = {
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+ { NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
+#include "llvm/Support/AArch64TargetParser.def"
};
} // namespace
@@ -369,6 +383,134 @@
return "generic";
}
+StringRef llvm::AArch64::getFPUName(unsigned FPUKind) {
+ return ARM::getFPUName(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUVersion(unsigned FPUKind) {
+ return ARM::getFPUVersion(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUNeonSupportLevel(unsigned FPUKind) {
+ return ARM::getFPUNeonSupportLevel( FPUKind);
+}
+
+unsigned llvm::AArch64::getFPURestriction(unsigned FPUKind) {
+ return ARM::getFPURestriction(FPUKind);
+}
+
+unsigned llvm::AArch64::getDefaultFPU(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].DefaultFPU;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, DEFAULT_FPU)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(ARM::FK_INVALID);
+}
+
+unsigned llvm::AArch64::getDefaultExtensions(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].ArchBaseExtensions;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, AArch64ARCHNames[ID].ArchBaseExtensions | DEFAULT_EXT)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(AArch64::AEK_INVALID);
+}
+
+bool llvm::AArch64::getExtensionFeatures(unsigned Extensions,
+ std::vector &Features) {
+
+ if (Extensions == AArch64::AEK_INVALID)
+return false;
+
+ if (Extensions & AArch64::AEK_FP)
+Features.push_back("+fp-armv8");
+ if (Extensions & AArch64::AEK_SIMD)
+Features.push_back("+neon");
+ if (Extensions & AArch64::AEK_CRC)
+Features.push_back("+crc");
+ if (Extensions & AArch64::AEK_CRYPTO)
+Features.push_back("+crypto");
+ if (Extensions & AArch64::AEK_FP16)
+Features.push_back("+fullfp16");
+ if (Extensions & AArch64::AEK_PROFILE)
+Features.push_back("+spe");
+
+ return true;
+}
+
+bool llvm::AArch64::getFPUFeatures(unsigned FPUKind,
+ std::vector &Features) {
+ return ARM::getFPUFeatures(FPUKind, Features);
+}
+
+StringRef llvm::AArch64::getArchName(unsigned ArchKind) {
+ if (ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getName();
+}
+
+StringRef llvm::AArch64::getCPUAttr(unsigned ArchKind) {
+ if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getCPUAttr();
+}
+
+StringRef llvm::AArch64::getSubArch(unsigned ArchKind) {
+ if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+retur
Re: [PATCH] D20088: Using AArch64TargetParser in clang
jojo removed rL LLVM as the repository for this revision.
jojo changed the visibility of this Differential Revision from "All Users" to
"Public (No Login Required)".
jojo updated this revision to Diff 58072.
jojo added a comment.
Remove checkARMArchValid & checkAArch64ArchValid logic.
http://reviews.llvm.org/D20088
Files:
lib/Basic/Targets.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -,22 +,9 @@
text.split(Split, StringRef("+"), -1, false);
for (StringRef Feature : Split) {
-const char *result = llvm::StringSwitch(Feature)
- .Case("fp", "+fp-armv8")
- .Case("simd", "+neon")
- .Case("crc", "+crc")
- .Case("crypto", "+crypto")
- .Case("fp16", "+fullfp16")
- .Case("profile", "+spe")
- .Case("nofp", "-fp-armv8")
- .Case("nosimd", "-neon")
- .Case("nocrc", "-crc")
- .Case("nocrypto", "-crypto")
- .Case("nofp16", "-fullfp16")
- .Case("noprofile", "-spe")
- .Default(nullptr);
-if (result)
- Features.push_back(result);
+const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature);
+if (FeatureName)
+ Features.push_back(FeatureName);
else if (Feature == "neon" || Feature == "noneon")
D.Diag(diag::err_drv_no_neon_modifier);
else
@@ -2252,20 +2239,16 @@
std::vector &Features) {
std::pair Split = Mcpu.split("+");
CPU = Split.first;
- if (CPU == "cortex-a53" || CPU == "cortex-a57" ||
- CPU == "cortex-a72" || CPU == "cortex-a35" || CPU == "exynos-m1" ||
- CPU == "kryo") {
-Features.push_back("+neon");
-Features.push_back("+crc");
-Features.push_back("+crypto");
- } else if (CPU == "cyclone") {
-Features.push_back("+neon");
-Features.push_back("+crypto");
- } else if (CPU == "generic") {
+
+ if (CPU == "generic") {
Features.push_back("+neon");
} else {
-return false;
- }
+unsigned ArchKind = llvm::AArch64::parseCPUArch(CPU);
+unsigned Extersion = llvm::AArch64::getDefaultExtensions(CPU,ArchKind);
+
+if (!llvm::AArch64::getExtensionFeatures(Extersion,Features))
+ return false;
+ }
if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
return false;
@@ -2280,17 +2263,8 @@
std::string MarchLowerCase = March.lower();
std::pair Split = StringRef(MarchLowerCase).split("+");
- if (Split.first == "armv8-a" || Split.first == "armv8a") {
-// ok, no additional features.
- } else if (Split.first == "armv8.1-a" || Split.first == "armv8.1a") {
-Features.push_back("+v8.1a");
- } else if (Split.first == "armv8.2-a" || Split.first == "armv8.2a" ) {
-Features.push_back("+v8.2a");
- } else {
-return false;
- }
-
- if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
+ if (llvm::AArch64::parseArch(Split.first, Features) == llvm::ARM::AK_INVALID ||
+ (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features)))
return false;
return true;
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5647,14 +5647,10 @@
}
bool setCPU(const std::string &Name) override {
-bool CPUKnown = llvm::StringSwitch(Name)
-.Case("generic", true)
-.Cases("cortex-a53", "cortex-a57", "cortex-a72",
- "cortex-a35", "exynos-m1", true)
-.Case("cyclone", true)
-.Case("kryo", true)
-.Default(false);
-return CPUKnown;
+if (Name == "generic" || llvm::AArch64::parseCPUArch(Name) != llvm::ARM::AK_INVALID)
+ return true;
+
+return false;
}
void getTargetDefines(const LangOptions &Opts,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20089: Adding a TargetParser for AArch64
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
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo set the repository for this revision to rL LLVM.
jojo changed the visibility of this Differential Revision from "Public (No
Login Required)" to "All Users".
jojo updated this revision to Diff 58205.
jojo added a comment.
1.include/llvm/Support/AArch64TargetParser.def
Format this file.
2.include/llvm/Support/TargetParser.h
Add FIXME and the definition of two methods
3.lib/Support/TargetParser.cpp
Adjust "llvm::AArch64::parseArch" logic.
Repository:
rL LLVM
http://reviews.llvm.org/D20089
Files:
include/llvm/Support/AArch64TargetParser.def
include/llvm/Support/ARMTargetParser.def
include/llvm/Support/TargetParser.h
lib/Support/TargetParser.cpp
Index: lib/Support/TargetParser.cpp
===
--- lib/Support/TargetParser.cpp
+++ lib/Support/TargetParser.cpp
@@ -21,6 +21,7 @@
using namespace llvm;
using namespace ARM;
+using namespace AArch64;
namespace {
@@ -75,6 +76,11 @@
{NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHNames[] = {
+#define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT) \
+ {NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
+ sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of Arch Extension names.
@@ -91,6 +97,10 @@
#define ARM_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
{ NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHExtNames[] = {
+#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
+ { NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of HWDiv names (use getHWDivSynonym) and which architectural
@@ -124,6 +134,10 @@
#define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
{ NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64CPUNames[] = {
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+ { NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
+#include "llvm/Support/AArch64TargetParser.def"
};
} // namespace
@@ -369,6 +383,153 @@
return "generic";
}
+StringRef llvm::AArch64::getFPUName(unsigned FPUKind) {
+ return ARM::getFPUName(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUVersion(unsigned FPUKind) {
+ return ARM::getFPUVersion(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUNeonSupportLevel(unsigned FPUKind) {
+ return ARM::getFPUNeonSupportLevel( FPUKind);
+}
+
+unsigned llvm::AArch64::getFPURestriction(unsigned FPUKind) {
+ return ARM::getFPURestriction(FPUKind);
+}
+
+unsigned llvm::AArch64::getDefaultFPU(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].DefaultFPU;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, DEFAULT_FPU)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(ARM::FK_INVALID);
+}
+
+unsigned llvm::AArch64::getDefaultExtensions(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].ArchBaseExtensions;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, AArch64ARCHNames[ID].ArchBaseExtensions | DEFAULT_EXT)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(AArch64::AEK_INVALID);
+}
+
+bool llvm::AArch64::getExtensionFeatures(unsigned Extensions,
+ std::vector &Features) {
+
+ if (Extensions == AArch64::AEK_INVALID)
+return false;
+
+ if (Extensions & AArch64::AEK_FP)
+Features.push_back("+fp-armv8");
+ if (Extensions & AArch64::AEK_SIMD)
+Features.push_back("+neon");
+ if (Extensions & AArch64::AEK_CRC)
+Features.push_back("+crc");
+ if (Extensions & AArch64::AEK_CRYPTO)
+Features.push_back("+crypto");
+ if (Extensions & AArch64::AEK_FP16)
+Features.push_back("+fullfp16");
+ if (Extensions & AArch64::AEK_PROFILE)
+Features.push_back("+spe");
+
+ return true;
+}
+
+bool llvm::AArch64::getFPUFeatures(unsigned FPUKind,
+ std::vector &Features) {
+ return ARM::getFPUFeatures(FPUKind, Features);
+}
+
+StringRef llvm::AArch64::getArchName(unsigned ArchKind) {
+ if (ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getName();
+}
+
+bool llvm::AArch64::getArchFeatures(unsigned ArchKind,
+ std::vector &Features) {
+ if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+return false;
+
+ if (ArchKind == ARM::AK_ARMV8_1A)
+Features.push_back("+v8.1a");
+ if (ArchKind == ARM::
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo added a comment. > While I agree with Bradley that the repetition is not pretty, I think it will > expose all issues to make a class design simple and straightforward, once we > get all the sharp edges out. But we need to know what are the difficulties on > Clang, llc and the back-ends, and make sure that the architectural part of > the change is correct, before we that. A class based design *will* change how every TargetParser method is being called now, and will touch a large number of files with mechanical changes, including in the ARM side, unrelated to the addition of AArch64. Yes.It will be a big project really,especially for me.I need to have a considerable understanding of the front-end, back-end, and the arm architecture, if I'm going to do it. > After my failed attempt at getting a class design across, I'd rather > introduce functionality first, then design better, than the other way round. > But I think they should be separated commits based on their intents and > merits alone. Totally agree.And that's exactly what I think.I think we can take class design as a low priority task.Maybe we could put it in our backlog list. 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
Re: [PATCH] D20088: Using AArch64TargetParser in clang
jojo set the repository for this revision to rL LLVM.
jojo changed the visibility of this Differential Revision from "Public (No
Login Required)" to "All Users".
jojo updated this revision to Diff 58210.
jojo added a comment.
Adjust "getAArch64ArchFeaturesFromMarch" logic.In file lib/Driver/Tools.cpp.
Repository:
rL LLVM
http://reviews.llvm.org/D20088
Files:
lib/Basic/Targets.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2223,22 +2223,9 @@
text.split(Split, StringRef("+"), -1, false);
for (StringRef Feature : Split) {
-const char *result = llvm::StringSwitch(Feature)
- .Case("fp", "+fp-armv8")
- .Case("simd", "+neon")
- .Case("crc", "+crc")
- .Case("crypto", "+crypto")
- .Case("fp16", "+fullfp16")
- .Case("profile", "+spe")
- .Case("nofp", "-fp-armv8")
- .Case("nosimd", "-neon")
- .Case("nocrc", "-crc")
- .Case("nocrypto", "-crypto")
- .Case("nofp16", "-fullfp16")
- .Case("noprofile", "-spe")
- .Default(nullptr);
-if (result)
- Features.push_back(result);
+const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature);
+if (FeatureName)
+ Features.push_back(FeatureName);
else if (Feature == "neon" || Feature == "noneon")
D.Diag(diag::err_drv_no_neon_modifier);
else
@@ -2253,20 +2240,16 @@
std::vector &Features) {
std::pair Split = Mcpu.split("+");
CPU = Split.first;
- if (CPU == "cortex-a53" || CPU == "cortex-a57" ||
- CPU == "cortex-a72" || CPU == "cortex-a35" || CPU == "exynos-m1" ||
- CPU == "kryo") {
-Features.push_back("+neon");
-Features.push_back("+crc");
-Features.push_back("+crypto");
- } else if (CPU == "cyclone") {
-Features.push_back("+neon");
-Features.push_back("+crypto");
- } else if (CPU == "generic") {
+
+ if (CPU == "generic") {
Features.push_back("+neon");
} else {
-return false;
- }
+unsigned ArchKind = llvm::AArch64::parseCPUArch(CPU);
+unsigned Extersion = llvm::AArch64::getDefaultExtensions(CPU,ArchKind);
+
+if (!llvm::AArch64::getExtensionFeatures(Extersion,Features))
+ return false;
+ }
if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
return false;
@@ -2278,20 +2261,13 @@
getAArch64ArchFeaturesFromMarch(const Driver &D, StringRef March,
const ArgList &Args,
std::vector &Features) {
+ unsigned ArchKind;
std::string MarchLowerCase = March.lower();
std::pair Split = StringRef(MarchLowerCase).split("+");
- if (Split.first == "armv8-a" || Split.first == "armv8a") {
-// ok, no additional features.
- } else if (Split.first == "armv8.1-a" || Split.first == "armv8.1a") {
-Features.push_back("+v8.1a");
- } else if (Split.first == "armv8.2-a" || Split.first == "armv8.2a" ) {
-Features.push_back("+v8.2a");
- } else {
-return false;
- }
-
- if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
+ ArchKind = llvm::AArch64::parseArch(Split.first);
+ if (ArchKind == llvm::ARM::AK_INVALID || !llvm::AArch64::getArchFeatures(ArchKind, Features) ||
+ (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features)))
return false;
return true;
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5665,14 +5665,10 @@
}
bool setCPU(const std::string &Name) override {
-bool CPUKnown = llvm::StringSwitch(Name)
-.Case("generic", true)
-.Cases("cortex-a53", "cortex-a57", "cortex-a72",
- "cortex-a35", "exynos-m1", true)
-.Case("cyclone", true)
-.Case("kryo", true)
-.Default(false);
-return CPUKnown;
+if (Name == "generic" || llvm::AArch64::parseCPUArch(Name) != llvm::ARM::AK_INVALID)
+ return true;
+
+return false;
}
void getTargetDefines(const LangOptions &Opts,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo updated this revision to Diff 58381.
jojo added a comment.
1.include/llvm/Support/TargetParser.h
Move move the declaration of getArchFeatures to a more reasonable place.
Remove unnecessary parameter for getDefaultCPU.
2.lib/Support/TargetParser.cpp
Make adjustments according to TargetParser.h.
Repository:
rL LLVM
http://reviews.llvm.org/D20089
Files:
include/llvm/Support/AArch64TargetParser.def
include/llvm/Support/ARMTargetParser.def
include/llvm/Support/TargetParser.h
lib/Support/TargetParser.cpp
Index: lib/Support/TargetParser.cpp
===
--- lib/Support/TargetParser.cpp
+++ lib/Support/TargetParser.cpp
@@ -21,6 +21,7 @@
using namespace llvm;
using namespace ARM;
+using namespace AArch64;
namespace {
@@ -75,6 +76,11 @@
{NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHNames[] = {
+#define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT) \
+ {NAME, sizeof(NAME) - 1, CPU_ATTR, sizeof(CPU_ATTR) - 1, SUB_ARCH, \
+ sizeof(SUB_ARCH) - 1, ARCH_FPU, ARCH_BASE_EXT, ID, ARCH_ATTR},
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of Arch Extension names.
@@ -91,6 +97,10 @@
#define ARM_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
{ NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64ARCHExtNames[] = {
+#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
+ { NAME, sizeof(NAME) - 1, ID, FEATURE, NEGFEATURE },
+#include "llvm/Support/AArch64TargetParser.def"
};
// List of HWDiv names (use getHWDivSynonym) and which architectural
@@ -124,6 +134,10 @@
#define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
{ NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
#include "llvm/Support/ARMTargetParser.def"
+},AArch64CPUNames[] = {
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+ { NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
+#include "llvm/Support/AArch64TargetParser.def"
};
} // namespace
@@ -369,6 +383,153 @@
return "generic";
}
+StringRef llvm::AArch64::getFPUName(unsigned FPUKind) {
+ return ARM::getFPUName(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUVersion(unsigned FPUKind) {
+ return ARM::getFPUVersion(FPUKind);
+}
+
+unsigned llvm::AArch64::getFPUNeonSupportLevel(unsigned FPUKind) {
+ return ARM::getFPUNeonSupportLevel( FPUKind);
+}
+
+unsigned llvm::AArch64::getFPURestriction(unsigned FPUKind) {
+ return ARM::getFPURestriction(FPUKind);
+}
+
+unsigned llvm::AArch64::getDefaultFPU(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].DefaultFPU;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, DEFAULT_FPU)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(ARM::FK_INVALID);
+}
+
+unsigned llvm::AArch64::getDefaultExtensions(StringRef CPU, unsigned ArchKind) {
+ if (CPU == "generic")
+return AArch64ARCHNames[ArchKind].ArchBaseExtensions;
+
+ return StringSwitch(CPU)
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+.Case(NAME, AArch64ARCHNames[ID].ArchBaseExtensions | DEFAULT_EXT)
+#include "llvm/Support/AArch64TargetParser.def"
+.Default(AArch64::AEK_INVALID);
+}
+
+bool llvm::AArch64::getExtensionFeatures(unsigned Extensions,
+ std::vector &Features) {
+
+ if (Extensions == AArch64::AEK_INVALID)
+return false;
+
+ if (Extensions & AArch64::AEK_FP)
+Features.push_back("+fp-armv8");
+ if (Extensions & AArch64::AEK_SIMD)
+Features.push_back("+neon");
+ if (Extensions & AArch64::AEK_CRC)
+Features.push_back("+crc");
+ if (Extensions & AArch64::AEK_CRYPTO)
+Features.push_back("+crypto");
+ if (Extensions & AArch64::AEK_FP16)
+Features.push_back("+fullfp16");
+ if (Extensions & AArch64::AEK_PROFILE)
+Features.push_back("+spe");
+
+ return true;
+}
+
+bool llvm::AArch64::getFPUFeatures(unsigned FPUKind,
+ std::vector &Features) {
+ return ARM::getFPUFeatures(FPUKind, Features);
+}
+
+bool llvm::AArch64::getArchFeatures(unsigned ArchKind,
+ std::vector &Features) {
+ if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+return false;
+
+ if (ArchKind == ARM::AK_ARMV8_1A)
+Features.push_back("+v8.1a");
+ if (ArchKind == ARM::AK_ARMV8_2A)
+Features.push_back("+v8.2a");
+
+ return true;
+}
+
+StringRef llvm::AArch64::getArchName(unsigned ArchKind) {
+ if (ArchKind >= ARM::AK_LAST)
+return StringRef();
+ return AArch64ARCHNames[ArchKind].getName();
+}
+
+StringRef llvm::AArch64::getCPUAttr(unsigned ArchKind) {
+ if (ArchKind == A
Re: [PATCH] D20089: Adding a TargetParser for AArch64
jojo added inline comments. Comment at: include/llvm/Support/TargetParser.h:173 @@ +172,3 @@ +StringRef getArchName(unsigned ArchKind); +bool getArchFeatures(unsigned ArchKind, std::vector &Features); +unsigned getArchAttr(unsigned ArchKind); rengolin wrote: > Nitpick, move this declaration with the others that use &Features. That would be more reasonable. Comment at: include/llvm/Support/TargetParser.h:184 @@ +183,3 @@ +unsigned getDefaultExtensions(StringRef CPU, unsigned ArchKind); +StringRef getDefaultCPU(StringRef Arch, std::vector &Features); + rengolin wrote: > You don't need the &Features any more. Same of the implementation in the cpp > file. Sorry, I missed it. 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
[PATCH] D21277: Resubmit r270688: Using new TargetParser in Clang.
jojo created this revision.
jojo added reviewers: rengolin, jmolloy, bsmith, rsmith, labrinea.
jojo added a subscriber: cfe-commits.
jojo set the repository for this revision to rL LLVM.
jojo changed the visibility of this Differential Revision from "Public (No
Login Required)" to "All Users".
Herald added a subscriber: aemerson.
Commit r270688: "Using AArch64TargetParser in clang "([[
http://reviews.llvm.org/D20088 | D20088 ]])broke some buildbots. This problem
could be fixed in [[ http://reviews.llvm.org/D21276 | D21276 ]],resubmit it.
It's almost no change,except for some rebase.
Repository:
rL LLVM
http://reviews.llvm.org/D21277
Files:
lib/Basic/Targets.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2299,24 +2299,9 @@
text.split(Split, StringRef("+"), -1, false);
for (StringRef Feature : Split) {
-const char *result = llvm::StringSwitch(Feature)
- .Case("fp", "+fp-armv8")
- .Case("simd", "+neon")
- .Case("crc", "+crc")
- .Case("crypto", "+crypto")
- .Case("fp16", "+fullfp16")
- .Case("profile", "+spe")
- .Case("ras", "+ras")
- .Case("nofp", "-fp-armv8")
- .Case("nosimd", "-neon")
- .Case("nocrc", "-crc")
- .Case("nocrypto", "-crypto")
- .Case("nofp16", "-fullfp16")
- .Case("noprofile", "-spe")
- .Case("noras", "-ras")
- .Default(nullptr);
-if (result)
- Features.push_back(result);
+const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature);
+if (FeatureName)
+ Features.push_back(FeatureName);
else if (Feature == "neon" || Feature == "noneon")
D.Diag(diag::err_drv_no_neon_modifier);
else
@@ -2331,20 +2316,16 @@
std::vector &Features) {
std::pair Split = Mcpu.split("+");
CPU = Split.first;
- if (CPU == "cortex-a53" || CPU == "cortex-a57" ||
- CPU == "cortex-a72" || CPU == "cortex-a35" || CPU == "exynos-m1" ||
- CPU == "kryo" || CPU == "cortex-a73") {
-Features.push_back("+neon");
-Features.push_back("+crc");
-Features.push_back("+crypto");
- } else if (CPU == "cyclone") {
-Features.push_back("+neon");
-Features.push_back("+crypto");
- } else if (CPU == "generic") {
+
+ if (CPU == "generic") {
Features.push_back("+neon");
} else {
-return false;
- }
+unsigned ArchKind = llvm::AArch64::parseCPUArch(CPU);
+unsigned Extersion = llvm::AArch64::getDefaultExtensions(CPU,ArchKind);
+
+if (!llvm::AArch64::getExtensionFeatures(Extersion,Features))
+ return false;
+ }
if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
return false;
@@ -2356,20 +2337,13 @@
getAArch64ArchFeaturesFromMarch(const Driver &D, StringRef March,
const ArgList &Args,
std::vector &Features) {
+ unsigned ArchKind;
std::string MarchLowerCase = March.lower();
std::pair Split = StringRef(MarchLowerCase).split("+");
- if (Split.first == "armv8-a" || Split.first == "armv8a") {
-// ok, no additional features.
- } else if (Split.first == "armv8.1-a" || Split.first == "armv8.1a") {
-Features.push_back("+v8.1a");
- } else if (Split.first == "armv8.2-a" || Split.first == "armv8.2a" ) {
-Features.push_back("+v8.2a");
- } else {
-return false;
- }
-
- if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
+ ArchKind = llvm::AArch64::parseArch(Split.first);
+ if (ArchKind == llvm::ARM::AK_INVALID || !llvm::AArch64::getArchFeatures(ArchKind, Features) ||
+ (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features)))
return false;
return true;
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5703,15 +5703,10 @@
}
bool setCPU(const std::string &Name) override {
-bool CPUKnown = llvm::StringSwitch(Name)
-.Case("generic", true)
-.Cases("cortex-a53", "cortex-a57", "cortex-a72",
- "cortex-a35", "exynos-m1", true)
-.Case("cortex-a73", true)
-.Case("cyclone", true)
-.Case("kryo", true)
-.Default(false);
-return CPUKnown;
+if (Name == "generic" || llvm::AArch64::parseCPUArch(Name) != llvm::ARM::AK_INVALID)
+ return true;
+
+return false;
}
void getTargetDe
Re: [PATCH] D21277: Resubmit r270688: Using new TargetParser in Clang.
jojo added inline comments. Comment at: lib/Basic/Targets.cpp:5709 @@ -5715,1 +5708,3 @@ + +return false; } echristo wrote: > compnerd wrote: > > Please collapse this: > > > > return Name == "generic" || llvm::AArch64::parseCPUArch(Name) != > > llvm::ARM::AK_INVALID; > ... why doesn't this actually set the cpu? Agree, this looks better. Comment at: lib/Basic/Targets.cpp:5709 @@ -5715,1 +5708,3 @@ + +return false; } jojo wrote: > echristo wrote: > > compnerd wrote: > > > Please collapse this: > > > > > > return Name == "generic" || llvm::AArch64::parseCPUArch(Name) != > > > llvm::ARM::AK_INVALID; > > ... why doesn't this actually set the cpu? > Agree, this looks better. Actually,I have no idea also. I just replaced the original implementation with the using of TargetParser. Comment at: lib/Driver/Tools.cpp:2303 @@ +2302,3 @@ +const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature); +if (FeatureName) + Features.push_back(FeatureName); compnerd wrote: > I think that you should use the scoped if here: > > if (const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature)) > Features.push_back(FeatureName); Agree,Looks more accurate and concise. Comment at: lib/Driver/Tools.cpp:2324 @@ +2323,3 @@ +unsigned ArchKind = llvm::AArch64::parseCPUArch(CPU); +unsigned Extersion = llvm::AArch64::getDefaultExtensions(CPU,ArchKind); + compnerd wrote: > NIT: Space after the comma. Thanks.I will correct it. Repository: rL LLVM http://reviews.llvm.org/D21277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21277: Resubmit r270688: Using new TargetParser in Clang.
jojo updated this revision to Diff 60658.
jojo added a comment.
Herald added a subscriber: mehdi_amini.
Update according to inline comments.
Repository:
rL LLVM
http://reviews.llvm.org/D21277
Files:
lib/Basic/Targets.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2299,24 +2299,8 @@
text.split(Split, StringRef("+"), -1, false);
for (StringRef Feature : Split) {
-const char *result = llvm::StringSwitch(Feature)
- .Case("fp", "+fp-armv8")
- .Case("simd", "+neon")
- .Case("crc", "+crc")
- .Case("crypto", "+crypto")
- .Case("fp16", "+fullfp16")
- .Case("profile", "+spe")
- .Case("ras", "+ras")
- .Case("nofp", "-fp-armv8")
- .Case("nosimd", "-neon")
- .Case("nocrc", "-crc")
- .Case("nocrypto", "-crypto")
- .Case("nofp16", "-fullfp16")
- .Case("noprofile", "-spe")
- .Case("noras", "-ras")
- .Default(nullptr);
-if (result)
- Features.push_back(result);
+if (const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature))
+ Features.push_back(FeatureName);
else if (Feature == "neon" || Feature == "noneon")
D.Diag(diag::err_drv_no_neon_modifier);
else
@@ -2331,20 +2315,16 @@
std::vector &Features) {
std::pair Split = Mcpu.split("+");
CPU = Split.first;
- if (CPU == "cortex-a53" || CPU == "cortex-a57" ||
- CPU == "cortex-a72" || CPU == "cortex-a35" || CPU == "exynos-m1" ||
- CPU == "kryo" || CPU == "cortex-a73") {
-Features.push_back("+neon");
-Features.push_back("+crc");
-Features.push_back("+crypto");
- } else if (CPU == "cyclone") {
-Features.push_back("+neon");
-Features.push_back("+crypto");
- } else if (CPU == "generic") {
+
+ if (CPU == "generic") {
Features.push_back("+neon");
} else {
-return false;
- }
+unsigned ArchKind = llvm::AArch64::parseCPUArch(CPU);
+unsigned Extersion = llvm::AArch64::getDefaultExtensions(CPU, ArchKind);
+
+if (!llvm::AArch64::getExtensionFeatures(Extersion, Features))
+ return false;
+ }
if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
return false;
@@ -2356,20 +2336,13 @@
getAArch64ArchFeaturesFromMarch(const Driver &D, StringRef March,
const ArgList &Args,
std::vector &Features) {
+ unsigned ArchKind;
std::string MarchLowerCase = March.lower();
std::pair Split = StringRef(MarchLowerCase).split("+");
- if (Split.first == "armv8-a" || Split.first == "armv8a") {
-// ok, no additional features.
- } else if (Split.first == "armv8.1-a" || Split.first == "armv8.1a") {
-Features.push_back("+v8.1a");
- } else if (Split.first == "armv8.2-a" || Split.first == "armv8.2a" ) {
-Features.push_back("+v8.2a");
- } else {
-return false;
- }
-
- if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
+ ArchKind = llvm::AArch64::parseArch(Split.first);
+ if (ArchKind == llvm::ARM::AK_INVALID || !llvm::AArch64::getArchFeatures(ArchKind, Features) ||
+ (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features)))
return false;
return true;
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5703,15 +5703,7 @@
}
bool setCPU(const std::string &Name) override {
-bool CPUKnown = llvm::StringSwitch(Name)
-.Case("generic", true)
-.Cases("cortex-a53", "cortex-a57", "cortex-a72",
- "cortex-a35", "exynos-m1", true)
-.Case("cortex-a73", true)
-.Case("cyclone", true)
-.Case("kryo", true)
-.Default(false);
-return CPUKnown;
+return Name == "generic" || llvm::AArch64::parseCPUArch(Name) != llvm::ARM::AK_INVALID;
}
void getTargetDefines(const LangOptions &Opts,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21277: Resubmit r270688: Using new TargetParser in Clang.
jojo added a comment. > I recommend you squash this patch with D21276 before commit. Hi, Renato, Do you mean updating the diff to let it include the change of http://reviews.llvm.org/D21276,or commiting these two reviews as one commit? Repository: rL LLVM http://reviews.llvm.org/D21277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21277: Resubmit r270688: Using new TargetParser in Clang.
jojo added a comment. I'm sorry that I should have submitted this and its fix in one review. Now the old fix http://reviews.llvm.org/D21276 is abandoned,as a latest fix is included in http://reviews.llvm.org/D21785. I will commit http://reviews.llvm.org/D21277 and http://reviews.llvm.org/D21785,once http://reviews.llvm.org/D21785 is approved. Repository: rL LLVM http://reviews.llvm.org/D21277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
