[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/tmatheson-arm commented: This looks like a great improvement over the existing system to me. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,104 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on. Does not change the base architecture, or follow dependencies
+ // between features which are only related by required arcitecture versions.
+ void enable(ArchExtKind E);
+
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it. Does not change the base architecture, or follow
+ // dependencies between features which are only related by required
+ // arcitecture versions.
+ void disable(ArchExtKind E);
+
+ // Add default extensions for the given CPU. Records the base architecture,
+ // to later resolve dependencies which depend on it.
+ void addCPUDefaults(const CpuInfo &CPU);
+
+ // Add default extensions for the given architecture version. Records the
+ // base architecture, to later resolve dependencies which depend on it.
+ void addArchDefaults(const ArchInfo &Arch);
+
+ // Add or remove a feature based on a modifier string. The string must be of
+ // the form "" to enable a feature or "no" to disable it. This
+ // will also enable or disable any features as required by the dependencies
+ // between them.
+ bool parseModifier(StringRef Modifier);
+
+ // Convert the set of enabled extension to an LLVM feature list, appending
+ // them to Features.
+ void toLLVMFeatureList(std::vector &Features) const;
+};
+
+// Represents a dependency between two architecture extensions. If Later is
+// enabled, then Earlier must also be enabled. If Earlier is disabled, then
+// Later must also be disabled.
+struct ExtensionDependency {
+ ArchExtKind Earlier;
+ ArchExtKind Later;
+};
+
+// clang-format off
+inline constexpr ExtensionDependency ExtensionDependencies[] = {
+ {AEK_FP, AEK_FP16},
DavidSpickett wrote:
Maybe a comment here "each entry here is a dependency chain starting from the
extension that was added to the architecture first", along those lines.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,104 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on. Does not change the base architecture, or follow dependencies
+ // between features which are only related by required arcitecture versions.
+ void enable(ArchExtKind E);
+
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it. Does not change the base architecture, or follow
+ // dependencies between features which are only related by required
+ // arcitecture versions.
+ void disable(ArchExtKind E);
+
+ // Add default extensions for the given CPU. Records the base architecture,
+ // to later resolve dependencies which depend on it.
+ void addCPUDefaults(const CpuInfo &CPU);
+
+ // Add default extensions for the given architecture version. Records the
+ // base architecture, to later resolve dependencies which depend on it.
+ void addArchDefaults(const ArchInfo &Arch);
+
+ // Add or remove a feature based on a modifier string. The string must be of
+ // the form "" to enable a feature or "no" to disable it. This
+ // will also enable or disable any features as required by the dependencies
+ // between them.
+ bool parseModifier(StringRef Modifier);
+
+ // Convert the set of enabled extension to an LLVM feature list, appending
+ // them to Features.
+ void toLLVMFeatureList(std::vector &Features) const;
+};
+
+// Represents a dependency between two architecture extensions. If Later is
+// enabled, then Earlier must also be enabled. If Earlier is disabled, then
+// Later must also be disabled.
DavidSpickett wrote:
You should clarify what kind of later this is. "Where feature Earlier was added
to the architecture before Later".
As opposed to later on the command line, added later in the compiler, etc.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
+ void enable(ArchExtKind E);
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it.
+ void disable(ArchExtKind E);
+
+ // Add default extensions for the given CPU.
+ void addCPUDefaults(const CpuInfo &CPU);
+ // Add default extensions for the given architecture version.
+ void addArchDefaults(const ArchInfo &Arch);
+
+ // Add or remove a feature based on a modifier string. The string must be of
+ // the form "" to enable a feature or "no" to disable it. This
+ // will also enable or disable any features as required by the dependencies
+ // between them.
+ bool parseModifier(StringRef Modifier);
+
+ // Convert the set of enabled extension to an LLVM feature list, appending
+ // them to Features.
+ void toLLVMFeatureList(std::vector &Features) const;
+};
+
+struct ExtensionDependency {
+ ArchExtKind Earlier;
+ ArchExtKind Later;
+};
+
+// clang-format off
+inline constexpr ExtensionDependency ExtensionDependencies[] = {
+ {AEK_FP, AEK_FP16},
DavidSpickett wrote:
I see, so each entry in this array is a chain from oldest to newest extension.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/jthackray approved this pull request. No specific comments, but am in favour of this, as it reduces code complexity and should ensure accuracy of features. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
+ void enable(ArchExtKind E);
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it.
+ void disable(ArchExtKind E);
+
+ // Add default extensions for the given CPU.
+ void addCPUDefaults(const CpuInfo &CPU);
+ // Add default extensions for the given architecture version.
+ void addArchDefaults(const ArchInfo &Arch);
+
+ // Add or remove a feature based on a modifier string. The string must be of
+ // the form "" to enable a feature or "no" to disable it. This
+ // will also enable or disable any features as required by the dependencies
+ // between them.
+ bool parseModifier(StringRef Modifier);
+
+ // Convert the set of enabled extension to an LLVM feature list, appending
+ // them to Features.
+ void toLLVMFeatureList(std::vector &Features) const;
DavidSpickett wrote:
Cool, leave it as is then.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
ostannard wrote:
It does not, I'll expand the comment.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
+ void enable(ArchExtKind E);
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it.
+ void disable(ArchExtKind E);
+
+ // Add default extensions for the given CPU.
+ void addCPUDefaults(const CpuInfo &CPU);
+ // Add default extensions for the given architecture version.
+ void addArchDefaults(const ArchInfo &Arch);
+
+ // Add or remove a feature based on a modifier string. The string must be of
+ // the form "" to enable a feature or "no" to disable it. This
+ // will also enable or disable any features as required by the dependencies
+ // between them.
+ bool parseModifier(StringRef Modifier);
+
+ // Convert the set of enabled extension to an LLVM feature list, appending
+ // them to Features.
+ void toLLVMFeatureList(std::vector &Features) const;
+};
+
+struct ExtensionDependency {
+ ArchExtKind Earlier;
+ ArchExtKind Later;
+};
+
+// clang-format off
+inline constexpr ExtensionDependency ExtensionDependencies[] = {
+ {AEK_FP, AEK_FP16},
ostannard wrote:
Yes, I picked the ordering to match the ordering in which extensions were added
to the architecture.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
+ void enable(ArchExtKind E);
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it.
DavidSpickett wrote:
Is this exhaustive? If `a` depends on `b` and `c`, I would get `a+b+c`. Then if
I disable `a`, am I left with `b+c`?
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
+ void enable(ArchExtKind E);
+ // Disable the given architecture extension, and any other extensions which
+ // depend on it.
+ void disable(ArchExtKind E);
+
+ // Add default extensions for the given CPU.
+ void addCPUDefaults(const CpuInfo &CPU);
+ // Add default extensions for the given architecture version.
+ void addArchDefaults(const ArchInfo &Arch);
+
+ // Add or remove a feature based on a modifier string. The string must be of
+ // the form "" to enable a feature or "no" to disable it. This
+ // will also enable or disable any features as required by the dependencies
+ // between them.
+ bool parseModifier(StringRef Modifier);
+
+ // Convert the set of enabled extension to an LLVM feature list, appending
+ // them to Features.
+ void toLLVMFeatureList(std::vector &Features) const;
DavidSpickett wrote:
Is it useful to have this return by ref? Could it be:
```
std::vector toLLVMFeatureList() const;
```
Depends what the majority of call sites do.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = {
};
// clang-format on
+struct ExtensionSet {
+ // Set of extensions which are currently enabled.
+ ExtensionBitset Enabled;
+ // Set of extensions which have been enabled or disabled at any point. Used
+ // to avoid cluttering the cc1 command-line with lots of unneeded features.
+ ExtensionBitset Touched;
+ // Base architecture version, which we need to know because some feature
+ // dependencies change depending on this.
+ const ArchInfo *BaseArch;
+
+ ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {}
+
+ // Enable the given architecture extension, and any other extensions it
+ // depends on.
DavidSpickett wrote:
Is it worth saying here whether it updates the base architecture version? I'm
guessing that it does not.
https://github.com/llvm/llvm-project/pull/78270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/DavidSpickett commented: Happy to see this change, will be a lot easier to reason about this once it's all in one place. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/sjoerdmeijer approved this pull request. This is a very big patch, but it makes a lot of sense. The idea to do it in this way is clearly an improvement. There are quite a number of (new) moving parts involved here, but look reasonable to me so let's give this is a try. Thanks for cleaning up the mess (also my mess). https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
