[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-20 Thread Tomas Matheson via cfe-commits 


@@ -56,43 +52,64 @@ class Extension<
 
 // The FMV priority
 int FMVPriority = _FMVPriority;
+
+// Indicates if the extension is available on the command line.
+string IsFMVOnly = _IsFMVOnly;
 }
 
 // Some extensions are available for FMV but can not be controlled via the
-// command line. These entries:
-//  - are SubtargetFeatures, so they have (unused) FieldNames on the subtarget
-//e.g. HasFMVOnlyFEAT_XYZ
-//  - have incorrect (empty) Implies fields, because the code that handles FMV
-//ignores these dependencies and looks only at FMVDependencies.
-//  - have no description.
-// 
-// In the generated data structures for extensions (ExtensionInfo), AEK_NONE is
-// used to indicate that a feature is FMV only. Therefore ArchExtKindSpelling 
is
-// manually overridden here.
-class FMVOnlyExtension
-  : Extension {
-let ArchExtKindSpelling = "AEK_NONE"; // AEK_NONE indicates FMV-only 
feature
-}
+// command line, neither have a TargetFeatureName. Since they have no effect
+// on their own, their description is left empty. However they can have side
+// effects by implying other Subtarget Features. These extensions are used
+// in FMV for detection purposes.
+
+let MArchName = "dgh" in
+def : Extension<"", "DGH", "", [], "FEAT_DGH", "", 260, "true">;

tmatheson-arm wrote:

> Then what makes an Extension with IsFMVOnly=1 different from and 
> FMVOnlyExtesion?

Nothing; I'm saying `FMVOnlyExtesion` remains a useful abstraction, even if the 
implementation of that concept is done with `IsFMVOnly==1` rather than 
`AEK_NONE`.

Specifically what I'm suggesting is:
- Introduce `IsFMVOnly` like you have done
- Keep `FMVOnlyExtension` as the only way to set `IsFMVOnly=1`
- Don't add a boolean parameter to the `Extension` constructor (or however it 
is termed in tablegen), because that will just encourage people to mess with it.

> Do you want to rename the FMVOnlyExtesion class into something else making it 
> differ from Extension only for not having a TargetFeatureName?

I don't follow. Currently the only difference between `FMVOnlyExtesion` and 
`Extension` is that `FMVOnlyExtesion` does not have a _`-march`_ name. They 
both have a `TargetFeatureName`, because both FMV extensions and 
`-march`/`target(...)` extensions have a `-target-feature` name.

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-18 Thread Alexandros Lamprineas via cfe-commits 


@@ -94,19 +94,21 @@ static void EmitARMTargetDef(RecordKeeper , raw_ostream 
) {
 else
   OS << ", \"" << Alias << "\"";
 OS << ", AArch64::" << AEK;
-if (AEK == "AEK_NONE") {
+auto Name = Rec->getValueAsString("Name");
+if (Name.empty()) {

labrinea wrote:

No, for example all the features I have fused  (FEAT_DPB, FEAT_DPB2, 
FEAT_FLAGM2, FEAT_FRINTTS, FEAT_RCPC2) as well as BTI, are FMVOnly and still 
have a valid +/- TargetFeatureName.

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-18 Thread Alexandros Lamprineas via cfe-commits 


@@ -56,43 +52,64 @@ class Extension<
 
 // The FMV priority
 int FMVPriority = _FMVPriority;
+
+// Indicates if the extension is available on the command line.
+string IsFMVOnly = _IsFMVOnly;

labrinea wrote:

We want the ExtensionInfo field to be of type bool. Can we do the type 
conversation in the emitter? If so okay

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-18 Thread Alexandros Lamprineas via cfe-commits 


@@ -56,43 +52,64 @@ class Extension<
 
 // The FMV priority
 int FMVPriority = _FMVPriority;
+
+// Indicates if the extension is available on the command line.
+string IsFMVOnly = _IsFMVOnly;
 }
 
 // Some extensions are available for FMV but can not be controlled via the
-// command line. These entries:
-//  - are SubtargetFeatures, so they have (unused) FieldNames on the subtarget
-//e.g. HasFMVOnlyFEAT_XYZ
-//  - have incorrect (empty) Implies fields, because the code that handles FMV
-//ignores these dependencies and looks only at FMVDependencies.
-//  - have no description.
-// 
-// In the generated data structures for extensions (ExtensionInfo), AEK_NONE is
-// used to indicate that a feature is FMV only. Therefore ArchExtKindSpelling 
is
-// manually overridden here.
-class FMVOnlyExtension
-  : Extension {
-let ArchExtKindSpelling = "AEK_NONE"; // AEK_NONE indicates FMV-only 
feature
-}
+// command line, neither have a TargetFeatureName. Since they have no effect
+// on their own, their description is left empty. However they can have side
+// effects by implying other Subtarget Features. These extensions are used
+// in FMV for detection purposes.
+
+let MArchName = "dgh" in
+def : Extension<"", "DGH", "", [], "FEAT_DGH", "", 260, "true">;

labrinea wrote:

Then what makes an Extension with IsFMVOnly=1 different from and 
FMVOnlyExtesion? I believe this will create more confusion. As I explained we 
don't want any extension to be of AEK_NONE type, in order to be able to express 
dependencies.

Do you want to rename the FMVOnlyExtesion class into something else making it 
differ from Extension only for not having a TargetFeatureName?

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread Tomas Matheson via cfe-commits 


@@ -56,43 +52,64 @@ class Extension<
 
 // The FMV priority
 int FMVPriority = _FMVPriority;
+
+// Indicates if the extension is available on the command line.
+string IsFMVOnly = _IsFMVOnly;

tmatheson-arm wrote:

```suggestion
bit IsFMVOnly = 0;
```

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread Tomas Matheson via cfe-commits 


@@ -94,19 +94,21 @@ static void EmitARMTargetDef(RecordKeeper , raw_ostream 
) {
 else
   OS << ", \"" << Alias << "\"";
 OS << ", AArch64::" << AEK;
-if (AEK == "AEK_NONE") {
+auto Name = Rec->getValueAsString("Name");
+if (Name.empty()) {

tmatheson-arm wrote:

Shouldn't we be checking `IsFMVOnly` here?

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread Tomas Matheson via cfe-commits 


@@ -56,43 +52,64 @@ class Extension<
 
 // The FMV priority
 int FMVPriority = _FMVPriority;
+
+// Indicates if the extension is available on the command line.
+string IsFMVOnly = _IsFMVOnly;
 }
 
 // Some extensions are available for FMV but can not be controlled via the
-// command line. These entries:
-//  - are SubtargetFeatures, so they have (unused) FieldNames on the subtarget
-//e.g. HasFMVOnlyFEAT_XYZ
-//  - have incorrect (empty) Implies fields, because the code that handles FMV
-//ignores these dependencies and looks only at FMVDependencies.
-//  - have no description.
-// 
-// In the generated data structures for extensions (ExtensionInfo), AEK_NONE is
-// used to indicate that a feature is FMV only. Therefore ArchExtKindSpelling 
is
-// manually overridden here.
-class FMVOnlyExtension
-  : Extension {
-let ArchExtKindSpelling = "AEK_NONE"; // AEK_NONE indicates FMV-only 
feature
-}
+// command line, neither have a TargetFeatureName. Since they have no effect
+// on their own, their description is left empty. However they can have side
+// effects by implying other Subtarget Features. These extensions are used
+// in FMV for detection purposes.
+
+let MArchName = "dgh" in
+def : Extension<"", "DGH", "", [], "FEAT_DGH", "", 260, "true">;

tmatheson-arm wrote:

I think this is a regression over using the `FMVOnlyExtension` class:
- These extensions are _not_ available via `-march`, but they require writing 
`let MArchName = "..."` while leaving their actual `Name` empty. That seems the 
wrong way around?
- The `FMVOnlyExtension` type makes it obvious what it is, whereas the boolean 
parameter makes it obscure.
- There are now subtarget fields like `HasDGH` which are not obviously 
FMV-only. Previously they were called `HasFMVOnlyDGH` etc.
- Removing the type and relying on convention leaves it open to abusing the 
abstraction, which as we have seen will happen very quickly.

This could be done while keeping the type:
```cpp
class FMVOnlyExtension
  : Extension<"", "FMVOnly"#FMVBit, "", [], FMVBit, Deps, Priority> {
let IsFMVOnly = 1;
// comment explaining why MArchName is overridden despite not working for 
-march
let MArchName = Name;
}
```

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread Alexandros Lamprineas via cfe-commits 

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

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-backend-aarch64

Author: Alexandros Lamprineas (labrinea)


Changes

When we moved the extension information into tablegen in #90987, some 
features (FEAT_DPB, FEAT_DPB2, FEAT_FLAGM2, FEAT_FRINTTS, FEAT_RCPC2) were 
defined as FMVOnlyExtension despite already having an equivalent 
SubtargetFeature in place. This patch is fusing these duplications.

---

Patch is 21.98 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/92319.diff


8 Files Affected:

- (modified) clang/lib/Basic/Targets/AArch64.cpp (+4-4) 
- (modified) clang/lib/CodeGen/CGBuiltin.cpp (+1-1) 
- (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+1-1) 
- (added) clang/test/Driver/aarch64-fmv-only-feature.c (+41) 
- (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+12-9) 
- (modified) llvm/lib/Target/AArch64/AArch64Features.td (+82-57) 
- (modified) llvm/lib/TargetParser/AArch64TargetParser.cpp (+12-1) 
- (modified) llvm/utils/TableGen/ARMTargetDefEmitter.cpp (+6-4) 


``diff
diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 5db1ce78c657f..ec9632045fa95 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -659,7 +659,7 @@ AArch64TargetInfo::getVScaleRange(const LangOptions 
) const {
 unsigned AArch64TargetInfo::multiVersionSortPriority(StringRef Name) const {
   if (Name == "default")
 return 0;
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->FmvPriority;
   return 0;
 }
@@ -670,13 +670,13 @@ unsigned AArch64TargetInfo::multiVersionFeatureCost() 
const {
 }
 
 bool AArch64TargetInfo::doesFeatureAffectCodeGen(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return !Ext->DependentFeatures.empty();
   return false;
 }
 
 StringRef AArch64TargetInfo::getFeatureDependencies(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->DependentFeatures;
   return StringRef();
 }
@@ -686,7 +686,7 @@ bool AArch64TargetInfo::validateCpuSupports(StringRef 
FeatureStr) const {
   llvm::SmallVector Features;
   FeatureStr.split(Features, "+");
   for (auto  : Features)
-if (!llvm::AArch64::parseArchExtension(Feature.trim()).has_value())
+if (!llvm::AArch64::parseFMVExtension(Feature.trim()).has_value())
   return false;
   return true;
 }
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f9ee93049b12d..ef0337d42505f 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14259,7 +14259,7 @@ Value *CodeGenFunction::EmitAArch64CpuSupports(const 
CallExpr *E) {
   ArgStr.split(Features, "+");
   for (auto  : Features) {
 Feature = Feature.trim();
-if (!llvm::AArch64::parseArchExtension(Feature))
+if (!llvm::AArch64::parseFMVExtension(Feature))
   return Builder.getFalse();
 if (Feature != "default")
   Features.push_back(Feature);
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index e32b060ebeb93..dcb517e978ca9 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -973,7 +973,7 @@ void AArch64ABIInfo::appendAttributeMangling(StringRef 
AttrStr,
 
   llvm::SmallDenseSet UniqueFeats;
   for (auto  : Features)
-if (auto Ext = llvm::AArch64::parseArchExtension(Feat))
+if (auto Ext = llvm::AArch64::parseFMVExtension(Feat))
   if (UniqueFeats.insert(Ext->Name).second)
 Out << 'M' << Ext->Name;
 }
diff --git a/clang/test/Driver/aarch64-fmv-only-feature.c 
b/clang/test/Driver/aarch64-fmv-only-feature.c
new file mode 100644
index 0..f918b3f8fb64a
--- /dev/null
+++ b/clang/test/Driver/aarch64-fmv-only-feature.c
@@ -0,0 +1,41 @@
+// Test that features which are meaningful only for Function Multiversioning 
are rejected from the command line.
+
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+dgh %s 2>&1 | 
FileCheck %s --check-prefix=DGH
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ebf16 %s 2>&1 | 
FileCheck %s --check-prefix=EBF16
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ls64_accdata %s 
2>&1 | FileCheck %s --check-prefix=LS64_ACCDATA
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ls64_v %s 2>&1 | 
FileCheck %s --check-prefix=LS64_V
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+memtag2 %s 2>&1 | 
FileCheck %s --check-prefix=MEMTAG2
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+memtag3 %s 2>&1 | 
FileCheck %s --check-prefix=MEMTAG3
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+pmull %s 2>&1 | 
FileCheck %s --check-prefix=PMULL
+// RUN: not %clang

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Alexandros Lamprineas (labrinea)


Changes

When we moved the extension information into tablegen in #90987, some 
features (FEAT_DPB, FEAT_DPB2, FEAT_FLAGM2, FEAT_FRINTTS, FEAT_RCPC2) were 
defined as FMVOnlyExtension despite already having an equivalent 
SubtargetFeature in place. This patch is fusing these duplications.

---

Patch is 21.98 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/92319.diff


8 Files Affected:

- (modified) clang/lib/Basic/Targets/AArch64.cpp (+4-4) 
- (modified) clang/lib/CodeGen/CGBuiltin.cpp (+1-1) 
- (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+1-1) 
- (added) clang/test/Driver/aarch64-fmv-only-feature.c (+41) 
- (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+12-9) 
- (modified) llvm/lib/Target/AArch64/AArch64Features.td (+82-57) 
- (modified) llvm/lib/TargetParser/AArch64TargetParser.cpp (+12-1) 
- (modified) llvm/utils/TableGen/ARMTargetDefEmitter.cpp (+6-4) 


``diff
diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 5db1ce78c657f..ec9632045fa95 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -659,7 +659,7 @@ AArch64TargetInfo::getVScaleRange(const LangOptions 
) const {
 unsigned AArch64TargetInfo::multiVersionSortPriority(StringRef Name) const {
   if (Name == "default")
 return 0;
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->FmvPriority;
   return 0;
 }
@@ -670,13 +670,13 @@ unsigned AArch64TargetInfo::multiVersionFeatureCost() 
const {
 }
 
 bool AArch64TargetInfo::doesFeatureAffectCodeGen(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return !Ext->DependentFeatures.empty();
   return false;
 }
 
 StringRef AArch64TargetInfo::getFeatureDependencies(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->DependentFeatures;
   return StringRef();
 }
@@ -686,7 +686,7 @@ bool AArch64TargetInfo::validateCpuSupports(StringRef 
FeatureStr) const {
   llvm::SmallVector Features;
   FeatureStr.split(Features, "+");
   for (auto  : Features)
-if (!llvm::AArch64::parseArchExtension(Feature.trim()).has_value())
+if (!llvm::AArch64::parseFMVExtension(Feature.trim()).has_value())
   return false;
   return true;
 }
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f9ee93049b12d..ef0337d42505f 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14259,7 +14259,7 @@ Value *CodeGenFunction::EmitAArch64CpuSupports(const 
CallExpr *E) {
   ArgStr.split(Features, "+");
   for (auto  : Features) {
 Feature = Feature.trim();
-if (!llvm::AArch64::parseArchExtension(Feature))
+if (!llvm::AArch64::parseFMVExtension(Feature))
   return Builder.getFalse();
 if (Feature != "default")
   Features.push_back(Feature);
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index e32b060ebeb93..dcb517e978ca9 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -973,7 +973,7 @@ void AArch64ABIInfo::appendAttributeMangling(StringRef 
AttrStr,
 
   llvm::SmallDenseSet UniqueFeats;
   for (auto  : Features)
-if (auto Ext = llvm::AArch64::parseArchExtension(Feat))
+if (auto Ext = llvm::AArch64::parseFMVExtension(Feat))
   if (UniqueFeats.insert(Ext->Name).second)
 Out << 'M' << Ext->Name;
 }
diff --git a/clang/test/Driver/aarch64-fmv-only-feature.c 
b/clang/test/Driver/aarch64-fmv-only-feature.c
new file mode 100644
index 0..f918b3f8fb64a
--- /dev/null
+++ b/clang/test/Driver/aarch64-fmv-only-feature.c
@@ -0,0 +1,41 @@
+// Test that features which are meaningful only for Function Multiversioning 
are rejected from the command line.
+
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+dgh %s 2>&1 | 
FileCheck %s --check-prefix=DGH
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ebf16 %s 2>&1 | 
FileCheck %s --check-prefix=EBF16
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ls64_accdata %s 
2>&1 | FileCheck %s --check-prefix=LS64_ACCDATA
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ls64_v %s 2>&1 | 
FileCheck %s --check-prefix=LS64_V
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+memtag2 %s 2>&1 | 
FileCheck %s --check-prefix=MEMTAG2
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+memtag3 %s 2>&1 | 
FileCheck %s --check-prefix=MEMTAG3
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+pmull %s 2>&1 | 
FileCheck %s --check-prefix=PMULL
+// RUN: not %clang --target=aarch64-linux-gnu

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: Alexandros Lamprineas (labrinea)


Changes

When we moved the extension information into tablegen in #90987, some 
features (FEAT_DPB, FEAT_DPB2, FEAT_FLAGM2, FEAT_FRINTTS, FEAT_RCPC2) were 
defined as FMVOnlyExtension despite already having an equivalent 
SubtargetFeature in place. This patch is fusing these duplications.

---

Patch is 21.98 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/92319.diff


8 Files Affected:

- (modified) clang/lib/Basic/Targets/AArch64.cpp (+4-4) 
- (modified) clang/lib/CodeGen/CGBuiltin.cpp (+1-1) 
- (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+1-1) 
- (added) clang/test/Driver/aarch64-fmv-only-feature.c (+41) 
- (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+12-9) 
- (modified) llvm/lib/Target/AArch64/AArch64Features.td (+82-57) 
- (modified) llvm/lib/TargetParser/AArch64TargetParser.cpp (+12-1) 
- (modified) llvm/utils/TableGen/ARMTargetDefEmitter.cpp (+6-4) 


``diff
diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 5db1ce78c657f..ec9632045fa95 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -659,7 +659,7 @@ AArch64TargetInfo::getVScaleRange(const LangOptions 
) const {
 unsigned AArch64TargetInfo::multiVersionSortPriority(StringRef Name) const {
   if (Name == "default")
 return 0;
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->FmvPriority;
   return 0;
 }
@@ -670,13 +670,13 @@ unsigned AArch64TargetInfo::multiVersionFeatureCost() 
const {
 }
 
 bool AArch64TargetInfo::doesFeatureAffectCodeGen(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return !Ext->DependentFeatures.empty();
   return false;
 }
 
 StringRef AArch64TargetInfo::getFeatureDependencies(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->DependentFeatures;
   return StringRef();
 }
@@ -686,7 +686,7 @@ bool AArch64TargetInfo::validateCpuSupports(StringRef 
FeatureStr) const {
   llvm::SmallVector Features;
   FeatureStr.split(Features, "+");
   for (auto  : Features)
-if (!llvm::AArch64::parseArchExtension(Feature.trim()).has_value())
+if (!llvm::AArch64::parseFMVExtension(Feature.trim()).has_value())
   return false;
   return true;
 }
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f9ee93049b12d..ef0337d42505f 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14259,7 +14259,7 @@ Value *CodeGenFunction::EmitAArch64CpuSupports(const 
CallExpr *E) {
   ArgStr.split(Features, "+");
   for (auto  : Features) {
 Feature = Feature.trim();
-if (!llvm::AArch64::parseArchExtension(Feature))
+if (!llvm::AArch64::parseFMVExtension(Feature))
   return Builder.getFalse();
 if (Feature != "default")
   Features.push_back(Feature);
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index e32b060ebeb93..dcb517e978ca9 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -973,7 +973,7 @@ void AArch64ABIInfo::appendAttributeMangling(StringRef 
AttrStr,
 
   llvm::SmallDenseSet UniqueFeats;
   for (auto  : Features)
-if (auto Ext = llvm::AArch64::parseArchExtension(Feat))
+if (auto Ext = llvm::AArch64::parseFMVExtension(Feat))
   if (UniqueFeats.insert(Ext->Name).second)
 Out << 'M' << Ext->Name;
 }
diff --git a/clang/test/Driver/aarch64-fmv-only-feature.c 
b/clang/test/Driver/aarch64-fmv-only-feature.c
new file mode 100644
index 0..f918b3f8fb64a
--- /dev/null
+++ b/clang/test/Driver/aarch64-fmv-only-feature.c
@@ -0,0 +1,41 @@
+// Test that features which are meaningful only for Function Multiversioning 
are rejected from the command line.
+
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+dgh %s 2>&1 | 
FileCheck %s --check-prefix=DGH
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ebf16 %s 2>&1 | 
FileCheck %s --check-prefix=EBF16
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ls64_accdata %s 
2>&1 | FileCheck %s --check-prefix=LS64_ACCDATA
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+ls64_v %s 2>&1 | 
FileCheck %s --check-prefix=LS64_V
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+memtag2 %s 2>&1 | 
FileCheck %s --check-prefix=MEMTAG2
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+memtag3 %s 2>&1 | 
FileCheck %s --check-prefix=MEMTAG3
+// RUN: not %clang --target=aarch64-linux-gnu -march=armv8-a+pmull %s 2>&1 | 
FileCheck %s --check-prefix=PMULL
+// RUN: not %clang --target=aarch64-linux-gnu

[clang] [llvm] [AArch64] Merge duplicate extension information. (PR #92319)

2024-05-17 Thread Alexandros Lamprineas via cfe-commits 

https://github.com/labrinea updated 
https://github.com/llvm/llvm-project/pull/92319

>From 0c00fc2537f9b6335aa35535ffaf09c57051f086 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas 
Date: Tue, 14 May 2024 17:46:00 +0100
Subject: [PATCH] [AArch64] Merge duplicate extension information.

When we moved the extension information into tablegen in #90987, some
features (FEAT_DPB, FEAT_DPB2, FEAT_FLAGM2, FEAT_FRINTTS, FEAT_RCPC2)
were defined as FMVOnlyExtension despite already having an equivalent
SubtargetFeature in place. This patch is fusing these duplications.

As a result these features are no longer AEK_NONE, which means they
would become available to the command line. Since we don't want that
I added a new field in ExtensionInfo to indicate whether a feature
IsFMVOnly. That made the class FMVOnlyExtension redundant so I have
removed it from tablegen.

To reject such features on the command line but have them accepted
in attribute strings we need two different parsing functions. I made
parseArchExtension skip over IsFMVOnly entries and created a new one
called parseFMVExtension which doesn't skip them.

Making all extensions have ArchExtKind != AEK_NONE is a stepping stone
towards deprecating DependentFeatures from ExtensionInfo completely,
since we will be able to express them through ExtensionDependency.
---
 clang/lib/Basic/Targets/AArch64.cpp   |   8 +-
 clang/lib/CodeGen/CGBuiltin.cpp   |   2 +-
 clang/lib/CodeGen/Targets/AArch64.cpp |   2 +-
 clang/test/Driver/aarch64-fmv-only-feature.c  |  41 ++
 .../llvm/TargetParser/AArch64TargetParser.h   |  21 +--
 llvm/lib/Target/AArch64/AArch64Features.td| 139 +++---
 llvm/lib/TargetParser/AArch64TargetParser.cpp |  13 +-
 llvm/utils/TableGen/ARMTargetDefEmitter.cpp   |  10 +-
 8 files changed, 159 insertions(+), 77 deletions(-)
 create mode 100644 clang/test/Driver/aarch64-fmv-only-feature.c

diff --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 5db1ce78c657f..ec9632045fa95 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -659,7 +659,7 @@ AArch64TargetInfo::getVScaleRange(const LangOptions 
) const {
 unsigned AArch64TargetInfo::multiVersionSortPriority(StringRef Name) const {
   if (Name == "default")
 return 0;
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->FmvPriority;
   return 0;
 }
@@ -670,13 +670,13 @@ unsigned AArch64TargetInfo::multiVersionFeatureCost() 
const {
 }
 
 bool AArch64TargetInfo::doesFeatureAffectCodeGen(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return !Ext->DependentFeatures.empty();
   return false;
 }
 
 StringRef AArch64TargetInfo::getFeatureDependencies(StringRef Name) const {
-  if (auto Ext = llvm::AArch64::parseArchExtension(Name))
+  if (auto Ext = llvm::AArch64::parseFMVExtension(Name))
 return Ext->DependentFeatures;
   return StringRef();
 }
@@ -686,7 +686,7 @@ bool AArch64TargetInfo::validateCpuSupports(StringRef 
FeatureStr) const {
   llvm::SmallVector Features;
   FeatureStr.split(Features, "+");
   for (auto  : Features)
-if (!llvm::AArch64::parseArchExtension(Feature.trim()).has_value())
+if (!llvm::AArch64::parseFMVExtension(Feature.trim()).has_value())
   return false;
   return true;
 }
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f9ee93049b12d..ef0337d42505f 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14259,7 +14259,7 @@ Value *CodeGenFunction::EmitAArch64CpuSupports(const 
CallExpr *E) {
   ArgStr.split(Features, "+");
   for (auto  : Features) {
 Feature = Feature.trim();
-if (!llvm::AArch64::parseArchExtension(Feature))
+if (!llvm::AArch64::parseFMVExtension(Feature))
   return Builder.getFalse();
 if (Feature != "default")
   Features.push_back(Feature);
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index e32b060ebeb93..dcb517e978ca9 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -973,7 +973,7 @@ void AArch64ABIInfo::appendAttributeMangling(StringRef 
AttrStr,
 
   llvm::SmallDenseSet UniqueFeats;
   for (auto  : Features)
-if (auto Ext = llvm::AArch64::parseArchExtension(Feat))
+if (auto Ext = llvm::AArch64::parseFMVExtension(Feat))
   if (UniqueFeats.insert(Ext->Name).second)
 Out << 'M' << Ext->Name;
 }
diff --git a/clang/test/Driver/aarch64-fmv-only-feature.c 
b/clang/test/Driver/aarch64-fmv-only-feature.c
new file mode 100644
index 0..f918b3f8fb64a
--- /dev/null
+++ b/clang/test/Driver/aarch64-fmv-only-feature.c
@@ -0,0 +1,41 @@
+// Test that features which are meaningful only for Function Multiversioning 
are rejected from the