[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread Anatoly Trosinenko via cfe-commits

atrosinenko wrote:

Meanwhile, which is current policy on enabling/disabling mandatory features via 
command line options?

https://github.com/llvm/llvm-project/pull/79311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread Anatoly Trosinenko via cfe-commits


@@ -860,7 +841,7 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector ,
 }
 if (Feature == "+dit")
   HasDIT = true;
-if (Feature == "+cccp")
+if (Feature == "+ccpp")

atrosinenko wrote:

Agree, but not yet sure how it is better to test this

https://github.com/llvm/llvm-project/pull/79311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread Anatoly Trosinenko via cfe-commits


@@ -741,7 +722,7 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector ,
 
 if (Feature == "+neon" || Feature == "+fp-armv8")
   FPU |= NeonMode;
-if (Feature == "+jscvt") {
+if (Feature == "+jsconv") {

atrosinenko wrote:

Agree, added a test case to `aarch64-target-features.c`

https://github.com/llvm/llvm-project/pull/79311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread Anatoly Trosinenko via cfe-commits

atrosinenko wrote:

I tried adding a few more lines to 
`clang/test/Preprocessor/aarch64-target-features.c` and turned out, clang 
accepts `-march=armv8-a+jscvt` but not `-march=armv8-a+dpb`. Brief debugging 
suggests this is because in the
```cpp
  {"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190},
```
line, `Feature` and `NegFeature` fields are empty.

I wonder if there is an overview of all the "feature-related" AArch64 stuff 
anywhere. As far as I understand, there are several kinds of identifiers:
* feature names defined in the chapter "A2: Armv8-A Architecture Extensions" of 
the Arm Architecture Reference Manual (such as `FEAT_JSCVT` mentioned in "A2.5: 
The Armv8.3 architecture extension")
* Function Multi-Versioning CPU features defined in `enum CPUFeatures` (such as 
`FEAT_JSCVT`)
* `AEK_*` constants defined in `enum ArchExtKind` (such as `AEK_JSCVT`)
* `Name`s defined in the `Extensions` array in AArch64TargetParser.h
* `Feature` / `NegFeature` defined in the same array
* `DependentFeatures` defined in the same array

I guess, each kind of identifiers belongs to one of logical "namespaces". Here 
is what I see:
* the names of FMV features looks identical to feature names defined in Arm 
ARM, but the _sets_ of identifiers are different - the former is a strict 
subset of the latter at best. Obviously, there is a number of features hardly 
related to compiler development (such as various properties of hardware debug 
interfaces) and some other are not related to runtime function selection by FMV 
machinery (or at least the rules are not yet written in ACLE for them). It is 
just a bit misleading that Arm ARM defines FEAT_PAUTH and this feature is 
mentioned as AEK_PAUTH, "+pauth" literal, etc. but not as FEAT_PAUTH in the 
enumeration with constants named `FEAT_*` :)
* after `AEK_*` constants were recently made not restricted by the bitsize of 
`uint64_t` anymore, these look like the best mirror of `FEAT_*` identifiers 
defined in Arm ARM (and there is an explicit [naming 
guideline](https://github.com/llvm/llvm-project/blob/7f409cd82b322038f08a984a07377758e76b0e4c/llvm/include/llvm/TargetParser/AArch64TargetParser.h#L106))
* `Name` fields are used when parsing `-march=...+feature` arguments of Clang. 
These are kind of user-visible interface
* `Feature`/`NegFeature` and `DependentFeatures` are used when parsing 
`-target-feature`s passed to `clang -cc1` and are likely to contain string 
identifiers from the same namespace, according to seemingly the [only in-tree 
user](https://github.com/llvm/llvm-project/blob/7fdb932c4e78706cec3468a1f149e5a54a865d36/clang/lib/Basic/Targets/AArch64.cpp#L1013)
 of `DependentFeatures` contents, but duplication like `{"jscvt", 
AArch64::AEK_JSCVT, "+jsconv", "-jsconv", FEAT_JSCVT, 
"+fp-armv8,+neon,+jsconv", 210}` (E.Feature additionally listed in 
E.DependentFeatures) looks redundant

https://github.com/llvm/llvm-project/pull/79311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread Anatoly Trosinenko via cfe-commits

https://github.com/atrosinenko updated 
https://github.com/llvm/llvm-project/pull/79311

>From e0d278d9fcdd01f574bbf5fa1bcbbaf7875525de Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko 
Date: Tue, 23 Jan 2024 12:40:00 +0300
Subject: [PATCH 1/3] [AArch64] Simplify Clang's description of architecture
 extensions

Core LLVM has AArch64TargetParser.h header describing the mapping from
Armv8.x and Armv9.x architecture extensions to the particular list of
features that are mandatory for the extension.

Clang partially reimplements this in AArch64TargetInfo::setArchFeatures()
function. This patch simplifies setArchFeatures() by dropping obvious
cases of duplication, though it seems not all the dependencies are
listed in AArch64TargetParser.h yet.

Specifically, this patch drops `HasX = true` statements for the
following features:
* v8.1-a: CRC, LSE, RDM
* v8.3-a: RCPC, NeonMode (it is enabled by the base Armv8-a and
  indirectly by -target-feature +fcma)
* v8.4-a: DOTPROD
* v8.6-a: BF16, MATMUL

In Clang's AArch64TargetInfo::handleTargetFeatures() function, the
string literal "+jscvt" is changed to "+jsconv" and "+cccp" to "+ccpp",
so the information is conveyed from TargetParser.

Additionally, this patch makes __ARM_FEATURE_BTI and __ARM_FEATURE_JCVT
conditional on the particular HasX flags instead of an architecture
extension as a whole, as using TargetParser makes it possible to specify
the requested architecture like "armv8.5+nojscvt".
---
 clang/lib/Basic/Targets/AArch64.cpp | 48 +
 1 file changed, 15 insertions(+), 33 deletions(-)

diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index d47181bfca4fc86..0ac890551fadb24 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -55,24 +55,19 @@ static constexpr Builtin::Info BuiltinInfo[] = {
 };
 
 void AArch64TargetInfo::setArchFeatures() {
+  // FIXME Could we drop this function altogether and migrate everything
+  //   to AArch64TargetParser in LLVM core?
   if (*ArchInfo == llvm::AArch64::ARMV8R) {
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
 FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
 HasLSE = true;
-HasRDM = true;
   } else if (ArchInfo->Version.getMajor() == 8) {
 if (ArchInfo->Version.getMinor() >= 7u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 6u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv8.6-a
 if (ArchInfo->Version.getMinor() >= 5u) {
   HasAlternativeNZCV = true;
   HasFRInt3264 = true;
@@ -82,30 +77,19 @@ void AArch64TargetInfo::setArchFeatures() {
   HasBTI = true;
 }
 if (ArchInfo->Version.getMinor() >= 4u) {
-  HasDotProd = true;
   HasDIT = true;
   HasFlagM = true;
 }
-if (ArchInfo->Version.getMinor() >= 3u) {
-  HasRCPC = true;
-  FPU |= NeonMode;
-}
+// No special cases for Armv8.3-a
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasCCPP = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasCRC = true;
-  HasLSE = true;
-  HasRDM = true;
-}
+// No special cases for Armv8.1-a
   } else if (ArchInfo->Version.getMajor() == 9) {
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv9.1-a
 FPU |= SveMode;
 HasSVE2 = true;
 HasFullFP16 = true;
@@ -115,15 +99,9 @@ void AArch64TargetInfo::setArchFeatures() {
 HasSB = true;
 HasPredRes = true;
 HasBTI = true;
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
-FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
-HasLSE = true;
-HasRDM = true;
   }
 }
 
@@ -257,7 +235,6 @@ void AArch64TargetInfo::getTargetDefinesARMV82A(const 
LangOptions ,
 void AArch64TargetInfo::getTargetDefinesARMV83A(const LangOptions ,
 MacroBuilder ) const {
   Builder.defineMacro("__ARM_FEATURE_COMPLEX", "1");
-  Builder.defineMacro("__ARM_FEATURE_JCVT", "1");
   Builder.defineMacro("__ARM_FEATURE_PAUTH", "1");
   // Also include the Armv8.2 defines
   getTargetDefinesARMV82A(Opts, Builder);
@@ -272,7 +249,6 @@ void AArch64TargetInfo::getTargetDefinesARMV84A(const 
LangOptions ,
 void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions ,
 MacroBuilder ) const {
   Builder.defineMacro("__ARM_FEATURE_FRINT", "1");
-  Builder.defineMacro("__ARM_FEATURE_BTI", "1");
   // Also include the Armv8.4 defines
   getTargetDefinesARMV84A(Opts, Builder);
 }
@@ -472,7 +448,10 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
,
   if ((FPU & NeonMode) && HasFullFP16)
 

[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread via cfe-commits


@@ -860,7 +841,7 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector ,
 }
 if (Feature == "+dit")
   HasDIT = true;
-if (Feature == "+cccp")
+if (Feature == "+ccpp")

ostannard wrote:

Again, does this need a test?

https://github.com/llvm/llvm-project/pull/79311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread via cfe-commits


@@ -741,7 +722,7 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector ,
 
 if (Feature == "+neon" || Feature == "+fp-armv8")
   FPU |= NeonMode;
-if (Feature == "+jscvt") {
+if (Feature == "+jsconv") {

ostannard wrote:

This looks like bug fix which would make a functional difference, does it need 
a test?

https://github.com/llvm/llvm-project/pull/79311
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Anatoly Trosinenko (atrosinenko)


Changes

Core LLVM has AArch64TargetParser.h header describing the mapping from Armv8.x 
and Armv9.x architecture extensions to the particular list of features that are 
mandatory for the extension.

Clang partially reimplements this in AArch64TargetInfo::setArchFeatures() 
function. This patch simplifies setArchFeatures() by dropping obvious cases of 
duplication, though it seems not all the dependencies are listed in 
AArch64TargetParser.h yet.

Specifically, this patch drops `HasX = true` statements for the following 
features:
* v8.1-a: CRC, LSE, RDM
* v8.3-a: RCPC, NeonMode (it is enabled by the base Armv8-a and indirectly by 
-target-feature +fcma)
* v8.4-a: DOTPROD
* v8.6-a: BF16, MATMUL

In Clang's AArch64TargetInfo::handleTargetFeatures() function, the string 
literal "+jscvt" is changed to "+jsconv" and "+cccp" to "+ccpp", so the 
information is conveyed from TargetParser.

Additionally, this patch makes __ARM_FEATURE_BTI and __ARM_FEATURE_JCVT 
conditional on the particular HasX flags instead of an architecture extension 
as a whole, as using TargetParser makes it possible to specify the requested 
architecture like "armv8.5+nojscvt".

---
Full diff: https://github.com/llvm/llvm-project/pull/79311.diff


1 Files Affected:

- (modified) clang/lib/Basic/Targets/AArch64.cpp (+15-34) 


``diff
diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index d47181bfca4fc8..76bb60db41099e 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -55,24 +55,18 @@ static constexpr Builtin::Info BuiltinInfo[] = {
 };
 
 void AArch64TargetInfo::setArchFeatures() {
+  // FIXME Could we drop this function altogether and migrate everything
+  //   to AArch64TargetParser in LLVM core?
   if (*ArchInfo == llvm::AArch64::ARMV8R) {
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
-FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
 HasLSE = true;
-HasRDM = true;
   } else if (ArchInfo->Version.getMajor() == 8) {
 if (ArchInfo->Version.getMinor() >= 7u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 6u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv8.6-a
 if (ArchInfo->Version.getMinor() >= 5u) {
   HasAlternativeNZCV = true;
   HasFRInt3264 = true;
@@ -82,30 +76,19 @@ void AArch64TargetInfo::setArchFeatures() {
   HasBTI = true;
 }
 if (ArchInfo->Version.getMinor() >= 4u) {
-  HasDotProd = true;
   HasDIT = true;
   HasFlagM = true;
 }
-if (ArchInfo->Version.getMinor() >= 3u) {
-  HasRCPC = true;
-  FPU |= NeonMode;
-}
+// No special cases for Armv8.3-a
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasCCPP = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasCRC = true;
-  HasLSE = true;
-  HasRDM = true;
-}
+// No special cases for Armv8.1-a
   } else if (ArchInfo->Version.getMajor() == 9) {
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv9.1-a
 FPU |= SveMode;
 HasSVE2 = true;
 HasFullFP16 = true;
@@ -115,15 +98,9 @@ void AArch64TargetInfo::setArchFeatures() {
 HasSB = true;
 HasPredRes = true;
 HasBTI = true;
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
-FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
-HasLSE = true;
-HasRDM = true;
   }
 }
 
@@ -257,7 +234,6 @@ void AArch64TargetInfo::getTargetDefinesARMV82A(const 
LangOptions ,
 void AArch64TargetInfo::getTargetDefinesARMV83A(const LangOptions ,
 MacroBuilder ) const {
   Builder.defineMacro("__ARM_FEATURE_COMPLEX", "1");
-  Builder.defineMacro("__ARM_FEATURE_JCVT", "1");
   Builder.defineMacro("__ARM_FEATURE_PAUTH", "1");
   // Also include the Armv8.2 defines
   getTargetDefinesARMV82A(Opts, Builder);
@@ -272,7 +248,6 @@ void AArch64TargetInfo::getTargetDefinesARMV84A(const 
LangOptions ,
 void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions ,
 MacroBuilder ) const {
   Builder.defineMacro("__ARM_FEATURE_FRINT", "1");
-  Builder.defineMacro("__ARM_FEATURE_BTI", "1");
   // Also include the Armv8.4 defines
   getTargetDefinesARMV84A(Opts, Builder);
 }
@@ -472,7 +447,10 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
,
   if ((FPU & NeonMode) && HasFullFP16)
 Builder.defineMacro("__ARM_FEATURE_FP16_VECTOR_ARITHMETIC", "1");
   if (HasFullFP16)
-   Builder.defineMacro("__ARM_FEATURE_FP16_SCALAR_ARITHMETIC", "1");
+Builder.defineMacro("__ARM_FEATURE_FP16_SCALAR_ARITHMETIC", 

[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-25 Thread Anatoly Trosinenko via cfe-commits

https://github.com/atrosinenko updated 
https://github.com/llvm/llvm-project/pull/79311

>From e0d278d9fcdd01f574bbf5fa1bcbbaf7875525de Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko 
Date: Tue, 23 Jan 2024 12:40:00 +0300
Subject: [PATCH 1/2] [AArch64] Simplify Clang's description of architecture
 extensions

Core LLVM has AArch64TargetParser.h header describing the mapping from
Armv8.x and Armv9.x architecture extensions to the particular list of
features that are mandatory for the extension.

Clang partially reimplements this in AArch64TargetInfo::setArchFeatures()
function. This patch simplifies setArchFeatures() by dropping obvious
cases of duplication, though it seems not all the dependencies are
listed in AArch64TargetParser.h yet.

Specifically, this patch drops `HasX = true` statements for the
following features:
* v8.1-a: CRC, LSE, RDM
* v8.3-a: RCPC, NeonMode (it is enabled by the base Armv8-a and
  indirectly by -target-feature +fcma)
* v8.4-a: DOTPROD
* v8.6-a: BF16, MATMUL

In Clang's AArch64TargetInfo::handleTargetFeatures() function, the
string literal "+jscvt" is changed to "+jsconv" and "+cccp" to "+ccpp",
so the information is conveyed from TargetParser.

Additionally, this patch makes __ARM_FEATURE_BTI and __ARM_FEATURE_JCVT
conditional on the particular HasX flags instead of an architecture
extension as a whole, as using TargetParser makes it possible to specify
the requested architecture like "armv8.5+nojscvt".
---
 clang/lib/Basic/Targets/AArch64.cpp | 48 +
 1 file changed, 15 insertions(+), 33 deletions(-)

diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index d47181bfca4fc86..0ac890551fadb24 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -55,24 +55,19 @@ static constexpr Builtin::Info BuiltinInfo[] = {
 };
 
 void AArch64TargetInfo::setArchFeatures() {
+  // FIXME Could we drop this function altogether and migrate everything
+  //   to AArch64TargetParser in LLVM core?
   if (*ArchInfo == llvm::AArch64::ARMV8R) {
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
 FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
 HasLSE = true;
-HasRDM = true;
   } else if (ArchInfo->Version.getMajor() == 8) {
 if (ArchInfo->Version.getMinor() >= 7u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 6u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv8.6-a
 if (ArchInfo->Version.getMinor() >= 5u) {
   HasAlternativeNZCV = true;
   HasFRInt3264 = true;
@@ -82,30 +77,19 @@ void AArch64TargetInfo::setArchFeatures() {
   HasBTI = true;
 }
 if (ArchInfo->Version.getMinor() >= 4u) {
-  HasDotProd = true;
   HasDIT = true;
   HasFlagM = true;
 }
-if (ArchInfo->Version.getMinor() >= 3u) {
-  HasRCPC = true;
-  FPU |= NeonMode;
-}
+// No special cases for Armv8.3-a
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasCCPP = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasCRC = true;
-  HasLSE = true;
-  HasRDM = true;
-}
+// No special cases for Armv8.1-a
   } else if (ArchInfo->Version.getMajor() == 9) {
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv9.1-a
 FPU |= SveMode;
 HasSVE2 = true;
 HasFullFP16 = true;
@@ -115,15 +99,9 @@ void AArch64TargetInfo::setArchFeatures() {
 HasSB = true;
 HasPredRes = true;
 HasBTI = true;
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
-FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
-HasLSE = true;
-HasRDM = true;
   }
 }
 
@@ -257,7 +235,6 @@ void AArch64TargetInfo::getTargetDefinesARMV82A(const 
LangOptions ,
 void AArch64TargetInfo::getTargetDefinesARMV83A(const LangOptions ,
 MacroBuilder ) const {
   Builder.defineMacro("__ARM_FEATURE_COMPLEX", "1");
-  Builder.defineMacro("__ARM_FEATURE_JCVT", "1");
   Builder.defineMacro("__ARM_FEATURE_PAUTH", "1");
   // Also include the Armv8.2 defines
   getTargetDefinesARMV82A(Opts, Builder);
@@ -272,7 +249,6 @@ void AArch64TargetInfo::getTargetDefinesARMV84A(const 
LangOptions ,
 void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions ,
 MacroBuilder ) const {
   Builder.defineMacro("__ARM_FEATURE_FRINT", "1");
-  Builder.defineMacro("__ARM_FEATURE_BTI", "1");
   // Also include the Armv8.4 defines
   getTargetDefinesARMV84A(Opts, Builder);
 }
@@ -472,7 +448,10 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
,
   if ((FPU & NeonMode) && HasFullFP16)
 

[clang] [AArch64] Simplify Clang's description of architecture extensions (PR #79311)

2024-01-24 Thread Anatoly Trosinenko via cfe-commits

https://github.com/atrosinenko created 
https://github.com/llvm/llvm-project/pull/79311

Core LLVM has AArch64TargetParser.h header describing the mapping from Armv8.x 
and Armv9.x architecture extensions to the particular list of features that are 
mandatory for the extension.

Clang partially reimplements this in AArch64TargetInfo::setArchFeatures() 
function. This patch simplifies setArchFeatures() by dropping obvious cases of 
duplication, though it seems not all the dependencies are listed in 
AArch64TargetParser.h yet.

Specifically, this patch drops `HasX = true` statements for the following 
features:
* v8.1-a: CRC, LSE, RDM
* v8.3-a: RCPC, NeonMode (it is enabled by the base Armv8-a and indirectly by 
-target-feature +fcma)
* v8.4-a: DOTPROD
* v8.6-a: BF16, MATMUL

In Clang's AArch64TargetInfo::handleTargetFeatures() function, the string 
literal "+jscvt" is changed to "+jsconv" and "+cccp" to "+ccpp", so the 
information is conveyed from TargetParser.

Additionally, this patch makes __ARM_FEATURE_BTI and __ARM_FEATURE_JCVT 
conditional on the particular HasX flags instead of an architecture extension 
as a whole, as using TargetParser makes it possible to specify the requested 
architecture like "armv8.5+nojscvt".

>From e0d278d9fcdd01f574bbf5fa1bcbbaf7875525de Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko 
Date: Tue, 23 Jan 2024 12:40:00 +0300
Subject: [PATCH] [AArch64] Simplify Clang's description of architecture
 extensions

Core LLVM has AArch64TargetParser.h header describing the mapping from
Armv8.x and Armv9.x architecture extensions to the particular list of
features that are mandatory for the extension.

Clang partially reimplements this in AArch64TargetInfo::setArchFeatures()
function. This patch simplifies setArchFeatures() by dropping obvious
cases of duplication, though it seems not all the dependencies are
listed in AArch64TargetParser.h yet.

Specifically, this patch drops `HasX = true` statements for the
following features:
* v8.1-a: CRC, LSE, RDM
* v8.3-a: RCPC, NeonMode (it is enabled by the base Armv8-a and
  indirectly by -target-feature +fcma)
* v8.4-a: DOTPROD
* v8.6-a: BF16, MATMUL

In Clang's AArch64TargetInfo::handleTargetFeatures() function, the
string literal "+jscvt" is changed to "+jsconv" and "+cccp" to "+ccpp",
so the information is conveyed from TargetParser.

Additionally, this patch makes __ARM_FEATURE_BTI and __ARM_FEATURE_JCVT
conditional on the particular HasX flags instead of an architecture
extension as a whole, as using TargetParser makes it possible to specify
the requested architecture like "armv8.5+nojscvt".
---
 clang/lib/Basic/Targets/AArch64.cpp | 48 +
 1 file changed, 15 insertions(+), 33 deletions(-)

diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index d47181bfca4fc86..0ac890551fadb24 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -55,24 +55,19 @@ static constexpr Builtin::Info BuiltinInfo[] = {
 };
 
 void AArch64TargetInfo::setArchFeatures() {
+  // FIXME Could we drop this function altogether and migrate everything
+  //   to AArch64TargetParser in LLVM core?
   if (*ArchInfo == llvm::AArch64::ARMV8R) {
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
 FPU |= NeonMode;
 HasCCPP = true;
-HasCRC = true;
 HasLSE = true;
-HasRDM = true;
   } else if (ArchInfo->Version.getMajor() == 8) {
 if (ArchInfo->Version.getMinor() >= 7u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 6u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv8.6-a
 if (ArchInfo->Version.getMinor() >= 5u) {
   HasAlternativeNZCV = true;
   HasFRInt3264 = true;
@@ -82,30 +77,19 @@ void AArch64TargetInfo::setArchFeatures() {
   HasBTI = true;
 }
 if (ArchInfo->Version.getMinor() >= 4u) {
-  HasDotProd = true;
   HasDIT = true;
   HasFlagM = true;
 }
-if (ArchInfo->Version.getMinor() >= 3u) {
-  HasRCPC = true;
-  FPU |= NeonMode;
-}
+// No special cases for Armv8.3-a
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasCCPP = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasCRC = true;
-  HasLSE = true;
-  HasRDM = true;
-}
+// No special cases for Armv8.1-a
   } else if (ArchInfo->Version.getMajor() == 9) {
 if (ArchInfo->Version.getMinor() >= 2u) {
   HasWFxT = true;
 }
-if (ArchInfo->Version.getMinor() >= 1u) {
-  HasBFloat16 = true;
-  HasMatMul = true;
-}
+// No special cases for Armv9.1-a
 FPU |= SveMode;
 HasSVE2 = true;
 HasFullFP16 = true;
@@ -115,15 +99,9 @@ void AArch64TargetInfo::setArchFeatures() {
 HasSB = true;
 HasPredRes = true;
 HasBTI = true;
-HasDotProd = true;
 HasDIT = true;
 HasFlagM = true;
-HasRCPC = true;
-FPU |= NeonMode;