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

Tomas Matheson via llvm-commits llvm-commits at lists.llvm.org
Mon May 20 05:39:33 PDT 2024


================
@@ -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<string FMVBit, string Name, string Deps, int Priority>
-  : Extension<Name, "FMVOnly"#FMVBit, "", [], FMVBit, Deps, Priority> {
-    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


More information about the llvm-commits mailing list