[llvm] [AMDGPU] Generalize generic-target numeric property validation (PR #223177)
Chinmay Deshpande via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 12 18:11:18 PDT 2026
https://github.com/chinmaydd updated https://github.com/llvm/llvm-project/pull/223177
>From ab0e486688e103b32a69d600833607a70373019e Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <chdeshpa at amd.com>
Date: Sat, 12 Sep 2026 13:55:12 -0400
Subject: [PATCH 1/2] [AMDGPU] Generalize generic-target numeric property
validation
Describe numeric properties with a field name, fallback value, and an
at-least, at-most, or equal comparison against each covered GPU. Keep exact
feature-record matching for unannotated frontend capabilities.
Share numeric evaluation and metadata defaults with table emission, while
rejecting conflicting values within a GPU. Reuse feature closures during
validation and emission.
Cover comparison policies, defaults, implied features, and malformed
metadata with synthetic TableGen tests.
Change-Id: I287dd5211ab542a7e8d03bd4a9eca4ac6460f588
---
llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td | 28 +-
.../AMDGPUTargetDefNumericFeatures.td | 278 ++++++++++++++++++
.../TableGen/Basic/AMDGPUTargetDefEmitter.cpp | 236 ++++++++++-----
3 files changed, 473 insertions(+), 69 deletions(-)
create mode 100644 llvm/test/TableGen/AMDGPUTargetDefNumericFeatures.td
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td b/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td
index 575baaf2ec0aa..1f8719f46b362 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td
@@ -6,11 +6,35 @@
//
//===----------------------------------------------------------------------===//
//
-// TargetParser metadata attached to processor records for the
-// -gen-amdgpu-target-def backend.
+// TargetParser metadata for the -gen-amdgpu-target-def backend.
//
//===----------------------------------------------------------------------===//
+// How a generic target's numeric property must compare with the same property
+// on each covered GPU. This only compares values between GPUs; it does not
+// relax the checks for conflicting feature values within a GPU.
+class AMDGPUGenericNumericComparison;
+def AMDGPUGenericAtLeast : AMDGPUGenericNumericComparison;
+def AMDGPUGenericAtMost : AMDGPUGenericNumericComparison;
+def AMDGPUGenericEqual : AMDGPUGenericNumericComparison;
+
+// Opt a numeric SubtargetFeature field into generic-target validation. All
+// features setting FieldName participate, including implied features and ones
+// that are not frontend-visible, and must agree on an integer-literal value.
+// DefaultValue is the fallback used by both validation and table emission when
+// no feature sets the field, and must match the backend's fallback. Declare at
+// most one property per FieldName.
+//
+// Fields without this metadata retain the exact feature-record matching used
+// for frontend-visible capabilities.
+class AMDGPUGenericNumericProperty<
+ string fieldName, int defaultValue,
+ AMDGPUGenericNumericComparison comparison> {
+ string FieldName = fieldName;
+ int DefaultValue = defaultValue;
+ AMDGPUGenericNumericComparison GenericComparison = comparison;
+}
+
// Marks a Processor/ProcessorModel record as a canonical GPU.
//
// \p isa is the ISA version [major, minor, stepping]. Empty for R600 (no AMDGCN
diff --git a/llvm/test/TableGen/AMDGPUTargetDefNumericFeatures.td b/llvm/test/TableGen/AMDGPUTargetDefNumericFeatures.td
new file mode 100644
index 0000000000000..000ad529ca6f9
--- /dev/null
+++ b/llvm/test/TableGen/AMDGPUTargetDefNumericFeatures.td
@@ -0,0 +1,278 @@
+// RUN: split-file %s %t
+// RUN: llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/valid.td \
+// RUN: | FileCheck %t/valid.td
+// RUN: llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/metadata-default.td \
+// RUN: | FileCheck %t/metadata-default.td
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/at-least-invalid.td 2>&1 \
+// RUN: | FileCheck %t/at-least-invalid.td -DFILE=%t/at-least-invalid.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/at-most-invalid.td 2>&1 \
+// RUN: | FileCheck %t/at-most-invalid.td -DFILE=%t/at-most-invalid.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/equal-invalid.td 2>&1 \
+// RUN: | FileCheck %t/equal-invalid.td -DFILE=%t/equal-invalid.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/missing-generic.td 2>&1 \
+// RUN: | FileCheck %t/missing-generic.td -DFILE=%t/missing-generic.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/missing-member.td 2>&1 \
+// RUN: | FileCheck %t/missing-member.td -DFILE=%t/missing-member.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/shared-implied.td 2>&1 \
+// RUN: | FileCheck %t/shared-implied.td -DFILE=%t/shared-implied.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/implied-value.td 2>&1 \
+// RUN: | FileCheck %t/implied-value.td -DFILE=%t/implied-value.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/generic-conflict.td 2>&1 \
+// RUN: | FileCheck %t/generic-conflict.td -DFILE=%t/generic-conflict.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/table-field-conflict.td 2>&1 \
+// RUN: | FileCheck %t/table-field-conflict.td -DFILE=%t/table-field-conflict.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/default-is-fallback.td 2>&1 \
+// RUN: | FileCheck %t/default-is-fallback.td -DFILE=%t/default-is-fallback.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/unannotated.td 2>&1 \
+// RUN: | FileCheck %t/unannotated.td -DFILE=%t/unannotated.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/invalid-value.td 2>&1 \
+// RUN: | FileCheck %t/invalid-value.td -DFILE=%t/invalid-value.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/duplicate-property.td 2>&1 \
+// RUN: | FileCheck %t/duplicate-property.td -DFILE=%t/duplicate-property.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/empty-field.td 2>&1 \
+// RUN: | FileCheck %t/empty-field.td -DFILE=%t/empty-field.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen -gen-amdgpu-target-def -I %t -I %p/../../include -I %p/../../lib/Target/AMDGPU %t/unknown-comparison.td 2>&1 \
+// RUN: | FileCheck %t/unknown-comparison.td -DFILE=%t/unknown-comparison.td --implicit-check-not="error:"
+
+//--- common.td
+include "llvm/Target/Target.td"
+include "AMDGPUTargetParser.td"
+def MyTarget : Target;
+
+class NumericFeature<string name, string fieldName, int value>
+ : SubtargetFeature<name, fieldName, !cast<string>(value), "">;
+
+def FeatureLow : NumericFeature<"low", "TestProperty", 4>;
+def FeatureHigh : NumericFeature<"high", "TestProperty", 8>;
+def FeatureHigher : NumericFeature<"higher", "TestProperty", 16>;
+def FeatureImpliesHigher : SubtargetFeature<
+ "implies-higher", "HasImpliesHigher", "true", "", [FeatureHigher]>;
+def FeatureCapacitySmall : NumericFeature<"capacity-small", "AddressableLocalMemorySize", 65536>;
+def FeatureCapacityBig : NumericFeature<"capacity-big", "AddressableLocalMemorySize", 131072>;
+def FeatureEncodingA : NumericFeature<"encoding-a", "Encoding", 7>;
+def FeatureEncodingB : NumericFeature<"encoding-b", "Encoding", 7>;
+
+// The higher value and the feature implying it are deliberately not visible.
+def AMDGPUFrontendVisibleFeatures {
+ list<SubtargetFeature> Features = [FeatureLow, FeatureHigh,
+ FeatureCapacitySmall, FeatureCapacityBig, FeatureEncodingA, FeatureEncodingB];
+}
+
+//--- valid.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+def CapacityProperty : AMDGPUGenericNumericProperty<
+ "AddressableLocalMemorySize", 32768, AMDGPUGenericAtMost>;
+def EncodingProperty : AMDGPUGenericNumericProperty<
+ "Encoding", 0, AMDGPUGenericEqual>;
+
+def FeatureWaves8 : NumericFeature<"waves-8", "MaxWavesPerEU", 8>;
+def FeatureWaves8Alias : NumericFeature<"waves-8-alias", "MaxWavesPerEU", 8>;
+def FeatureWaves16 : NumericFeature<"waves-16", "MaxWavesPerEU", 16>;
+
+// Multiple features setting the same value are still accepted.
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel,
+ [FeatureLow, FeatureCapacitySmall, FeatureEncodingA, FeatureWaves8,
+ FeatureWaves8Alias], [9, 0, 0]>;
+def GFX901 : AMDGPUProcessorModel<"gfx901", NoSchedModel,
+ [FeatureHigh, FeatureCapacityBig, FeatureEncodingB, FeatureWaves16], [9, 0, 1]>;
+def GFX902 : AMDGPUProcessorModel<"gfx902", NoSchedModel, [], [9, 0, 2]>;
+def GFX903 : AMDGPUProcessorModel<"gfx903", NoSchedModel,
+ [FeatureImpliesHigher], [9, 0, 3]>;
+
+// All three comparisons replace exact-record matching for visible numeric
+// features. In particular, the generic's encoding-a is absent from GFX901,
+// whose encoding-b sets the same value, and its capacity-small is below GFX901's
+// capacity-big. Neither prevents this generic from covering GFX901.
+// CHECK: GK_GFX9_GENERIC
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel,
+ [FeatureHigh, FeatureCapacitySmall, FeatureEncodingA], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900, GFX901];
+}
+
+// Missing features use the declared defaults on either side.
+// CHECK: GK_GFX8_GENERIC
+def : AMDGPUProcessorModel<"gfx8-generic", NoSchedModel, [], [8, 0, 0]> {
+ let CoveredGPUs = [GFX902];
+}
+
+// Follow Implies edges, including features outside the frontend-visible set.
+// CHECK: GK_GFX7_GENERIC
+def : AMDGPUProcessorModel<"gfx7-generic", NoSchedModel,
+ [FeatureHigher], [7, 0, 0]> {
+ let CoveredGPUs = [GFX903];
+}
+
+// The emitted table uses the unambiguous feature value, or its fallback when no
+// feature sets the field. A fallback (MaxWavesPerEU = 10) is not a minimum.
+// CHECK: {[[#]], Triple::AMDGPUSubArch900, {{.*}}, {9, 0, 0}, [[#]], 8, 65536, 32, 0},
+// CHECK: {[[#]], Triple::AMDGPUSubArch901, {{.*}}, {9, 0, 1}, [[#]], 16, 131072, 32, 0},
+// CHECK: {[[#]], Triple::AMDGPUSubArch902, {{.*}}, {9, 0, 2}, [[#]], 10, 32768, 32, 0},
+
+//--- metadata-default.td
+include "common.td"
+// Deliberately differ from the legacy 32768 fallback. The metadata must be used
+// both when checking equality and when emitting concrete and generic GPU rows.
+def CapacityProperty : AMDGPUGenericNumericProperty<
+ "AddressableLocalMemorySize", 65536, AMDGPUGenericEqual>;
+def FeatureCapacity32K : NumericFeature<"capacity-32k", "AddressableLocalMemorySize", 32768>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel,
+ [FeatureCapacitySmall], [9, 0, 0]>;
+def GFX901 : AMDGPUProcessorModel<"gfx901", NoSchedModel, [], [9, 0, 1]>;
+// An explicit value below the metadata fallback must not be clamped to it.
+def GFX902 : AMDGPUProcessorModel<"gfx902", NoSchedModel,
+ [FeatureCapacity32K], [9, 0, 2]>;
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900, GFX901];
+}
+// CHECK: {[[#]], Triple::AMDGPUSubArch900, {{.*}}, {9, 0, 0}, [[#]], 10, 65536, 32, 0},
+// CHECK: {[[#]], Triple::AMDGPUSubArch901, {{.*}}, {9, 0, 1}, [[#]], 10, 65536, 32, 0},
+// CHECK: {[[#]], Triple::AMDGPUSubArch902, {{.*}}, {9, 0, 2}, [[#]], 10, 32768, 32, 0},
+// CHECK: {[[#]], Triple::AMDGPUSubArch9, {{.*}}, {9, 0, 0}, [[#]], 10, 65536, 32, 0},
+
+//--- at-least-invalid.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureHigh], [9, 0, 0]>;
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 4, which must be at least the value 8 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- at-most-invalid.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtMost>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureLow], [9, 0, 0]>;
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 8, which must be at most the value 4 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureHigh], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- equal-invalid.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericEqual>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureHigh], [9, 0, 0]>;
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 4, which must be equal to the value 8 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- missing-generic.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureHigh], [9, 0, 0]>;
+// A missing generic feature still requires checking the property's default.
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 4, which must be at least the value 8 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- missing-member.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtMost>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [], [9, 0, 0]>;
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 8, which must be at most the value 4 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureHigh], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- shared-implied.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+// A shared visible feature cannot hide a conflicting implied, non-visible one.
+// CHECK: [[FILE]]:[[#@LINE+1]]:5: error: GPU 'gfx900' gets conflicting 'TestProperty' values from 'higher' and 'low'
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel,
+ [FeatureLow, FeatureImpliesHigher], [9, 0, 0]>;
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- implied-value.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel,
+ [FeatureImpliesHigher], [9, 0, 0]>;
+// The numeric comparison also considers implied features that are not visible.
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 4, which must be at least the value 16 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- generic-conflict.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureHigh], [9, 0, 0]>;
+// Reject conflicting values in the generic target as well as in its members.
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: GPU 'gfx9-generic' gets conflicting 'TestProperty' values from 'low' and 'high'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel,
+ [FeatureHigh, FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- table-field-conflict.td
+include "common.td"
+def FeatureWaves8 : NumericFeature<"waves-8", "MaxWavesPerEU", 8>;
+def FeatureWaves16 : NumericFeature<"waves-16", "MaxWavesPerEU", 16>;
+// Preserve conflict checks for existing table fields, even without property
+// metadata or any generic target covering this GPU.
+// CHECK: [[FILE]]:[[#@LINE+1]]:5: error: GPU 'gfx900' gets conflicting 'MaxWavesPerEU' values from 'waves-16' and 'waves-8'
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel,
+ [FeatureWaves8, FeatureWaves16], [9, 0, 0]>;
+
+//--- default-is-fallback.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 10, AMDGPUGenericAtMost>;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureLow], [9, 0, 0]>;
+// The default does not clamp explicitly specified values up to 10.
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' has 'TestProperty' value 8, which must be at most the value 4 of covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureHigh], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- unannotated.td
+include "common.td"
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureHigh], [9, 0, 0]>;
+// Numeric-looking values alone do not opt a field out of exact matching.
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: generic target 'gfx9-generic' exposes feature 'low' not supported by covered GPU 'gfx900'
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- invalid-value.td
+include "common.td"
+def TestProperty : AMDGPUGenericNumericProperty<
+ "TestProperty", 4, AMDGPUGenericAtLeast>;
+// Numeric validation requires integer literals, not C++ enum expressions.
+// CHECK: [[FILE]]:[[#@LINE+1]]:5: error: feature 'invalid' must have an integer value
+def FeatureInvalid : SubtargetFeature<"invalid", "TestProperty", "SomeEnumValue", "">;
+def GFX900 : AMDGPUProcessorModel<"gfx900", NoSchedModel, [FeatureInvalid], [9, 0, 0]>;
+def : AMDGPUProcessorModel<"gfx9-generic", NoSchedModel, [FeatureLow], [9, 0, 0]> {
+ let CoveredGPUs = [GFX900];
+}
+
+//--- duplicate-property.td
+include "common.td"
+def A : AMDGPUGenericNumericProperty<"TestProperty", 4, AMDGPUGenericAtLeast>;
+// CHECK: [[FILE]]:[[#@LINE+1]]:5: error: duplicate numeric property for field 'TestProperty'
+def B : AMDGPUGenericNumericProperty<"TestProperty", 4, AMDGPUGenericAtMost>;
+
+//--- empty-field.td
+include "common.td"
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: numeric property must have a field name
+def : AMDGPUGenericNumericProperty<"", 4, AMDGPUGenericAtLeast>;
+
+//--- unknown-comparison.td
+include "common.td"
+def UnknownComparison : AMDGPUGenericNumericComparison;
+// CHECK: [[FILE]]:[[#@LINE+1]]:1: error: unknown generic numeric comparison 'UnknownComparison'
+def : AMDGPUGenericNumericProperty<"TestProperty", 4, UnknownComparison>;
diff --git a/llvm/utils/TableGen/Basic/AMDGPUTargetDefEmitter.cpp b/llvm/utils/TableGen/Basic/AMDGPUTargetDefEmitter.cpp
index 69da1d25896fb..3dfcc339cd29b 100644
--- a/llvm/utils/TableGen/Basic/AMDGPUTargetDefEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/AMDGPUTargetDefEmitter.cpp
@@ -11,6 +11,7 @@
//
//===----------------------------------------------------------------------===//
+#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallString.h"
@@ -188,7 +189,7 @@ collectFrontendFeatures(const RecordKeeper &RK, StringRef ListName) {
static void
emitFeatureBitset(raw_ostream &OS, StringRef BitsetType, StringRef EnumPrefix,
- const Record *GPU,
+ ArrayRef<const Record *> Closure,
const DenseMap<const Record *, unsigned> &FeatureIdx);
// The transitive closure of a GPU's SubtargetFeatures, following the Implies
@@ -308,9 +309,12 @@ emitR600Table(raw_ostream &OS, const RecordKeeper &RK,
OS << ";\n"
"static constexpr R600Info R600GPUTable[] = {\n";
for (const Record *R : Canon) {
+ SetVector<const Record *> Closure;
+ collectFeatureClosure(R, Closure);
OS << " {" << Names.GetOrAddStringOffset(R->getValueAsString("Name"))
<< ", ";
- emitFeatureBitset(OS, "R600FeatureBitset", "R600_FEAT_", R, FeatureIdx);
+ emitFeatureBitset(OS, "R600FeatureBitset", "R600_FEAT_",
+ Closure.getArrayRef(), FeatureIdx);
OS << "},\n";
}
OS << "};\n"
@@ -463,10 +467,8 @@ static void emitFeatureNames(raw_ostream &OS, const FeatureNaming &Naming,
// The set of frontend features that end up in the emitted bitset.
static SetVector<const Record *>
-collectVisibleFeatures(const Record *GPU,
+collectVisibleFeatures(ArrayRef<const Record *> Closure,
const DenseMap<const Record *, unsigned> &FeatureIdx) {
- SetVector<const Record *> Closure;
- collectFeatureClosure(GPU, Closure);
SetVector<const Record *> Visible;
for (const Record *F : Closure) {
if (FeatureIdx.contains(F))
@@ -476,48 +478,174 @@ collectVisibleFeatures(const Record *GPU,
return Visible;
}
+namespace {
+enum class GenericNumericComparison { AtLeast, AtMost, Equal };
+
+struct NumericProperty {
+ int64_t DefaultValue;
+ GenericNumericComparison Comparison;
+};
+
+using NumericPropertyMap = MapVector<StringRef, NumericProperty>;
+} // namespace
+
+// The value of the features setting FieldName in GPU's closure. Preserve the
+// check that such features agree: some backend consumers inspect individual
+// feature bits instead of using SubtargetFeature's maximum-value rule.
+// If no feature sets the field, use its property default, or
+// UnregisteredDefault for fields without metadata.
+static int64_t getFeatureValue(const Record *GPU,
+ ArrayRef<const Record *> Closure,
+ StringRef FieldName,
+ const NumericPropertyMap &Properties,
+ int64_t UnregisteredDefault = 0) {
+ auto It = Properties.find(FieldName);
+ int64_t Value =
+ It == Properties.end() ? UnregisteredDefault : It->second.DefaultValue;
+
+ const Record *Found = nullptr;
+ for (const Record *F : Closure) {
+ if (F->getValueAsString("FieldName") != FieldName)
+ continue;
+
+ int64_t V;
+ if (!to_integer(F->getValueAsString("Value"), V)) {
+ PrintFatalError(F->getLoc(), "feature '" + F->getValueAsString("Name") +
+ "' must have an integer value");
+ }
+ if (Found && V != Value) {
+ PrintFatalError(GPU->getLoc(),
+ "GPU '" + GPU->getValueAsString("Name") +
+ "' gets conflicting '" + FieldName +
+ "' values from '" + Found->getValueAsString("Name") +
+ "' and '" + F->getValueAsString("Name") + "'");
+ }
+ Found = F;
+ Value = V;
+ }
+ return Value;
+}
+
+static NumericPropertyMap collectNumericProperties(const RecordKeeper &RK) {
+ NumericPropertyMap Properties;
+ for (const Record *R :
+ RK.getAllDerivedDefinitionsIfDefined("AMDGPUGenericNumericProperty")) {
+ StringRef FieldName = R->getValueAsString("FieldName");
+ if (FieldName.empty())
+ PrintFatalError(R->getLoc(), "numeric property must have a field name");
+
+ StringRef ComparisonName = R->getValueAsDef("GenericComparison")->getName();
+ GenericNumericComparison Comparison;
+ if (ComparisonName == "AMDGPUGenericAtLeast")
+ Comparison = GenericNumericComparison::AtLeast;
+ else if (ComparisonName == "AMDGPUGenericAtMost")
+ Comparison = GenericNumericComparison::AtMost;
+ else if (ComparisonName == "AMDGPUGenericEqual")
+ Comparison = GenericNumericComparison::Equal;
+ else
+ PrintFatalError(R->getLoc(), "unknown generic numeric comparison '" +
+ ComparisonName + "'");
+
+ NumericProperty Property{R->getValueAsInt("DefaultValue"), Comparison};
+ if (!Properties.insert({FieldName, Property}).second)
+ PrintFatalError(R->getLoc(), "duplicate numeric property for field '" +
+ FieldName + "'");
+ }
+ return Properties;
+}
+
+static void validateGenericNumericProperties(
+ const Record *GPU, ArrayRef<int64_t> GenericValues, const Record *Member,
+ ArrayRef<const Record *> MemberClosure,
+ const NumericPropertyMap &Properties) {
+ for (const auto &[Entry, GenericValue] :
+ zip_equal(Properties, GenericValues)) {
+ const auto &[FieldName, Property] = Entry;
+ int64_t MemberValue =
+ getFeatureValue(Member, MemberClosure, FieldName, Properties);
+ StringRef RequiredRelation;
+ switch (Property.Comparison) {
+ case GenericNumericComparison::AtLeast:
+ if (GenericValue >= MemberValue)
+ continue;
+ RequiredRelation = "at least";
+ break;
+ case GenericNumericComparison::AtMost:
+ if (GenericValue <= MemberValue)
+ continue;
+ RequiredRelation = "at most";
+ break;
+ case GenericNumericComparison::Equal:
+ if (GenericValue == MemberValue)
+ continue;
+ RequiredRelation = "equal to";
+ break;
+ }
+ PrintFatalError(
+ GPU->getLoc(),
+ "generic target '" + GPU->getValueAsString("Name") + "' has '" +
+ FieldName + "' value " + Twine(GenericValue) + ", which must be " +
+ RequiredRelation + " the value " + Twine(MemberValue) +
+ " of covered GPU '" + Member->getValueAsString("Name") + "'");
+ }
+}
+
// Make sure a "gfxN-generic" processor doesn't expose a frontend-visible
-// feature missing from any covered processor.
+// feature missing from any covered processor. Numeric properties with explicit
+// metadata are checked by field value instead of exact feature-record matching.
//
-// FIXME: The check should cover all SubtargetFeatures, not just the
+// FIXME: The capability check should cover all SubtargetFeatures, not just the
// frontend-visible ones. It is limited to those because a generic legitimately
-// carries some features a covered GPU lacks (bug/hazard workarounds and
-// worst-case-valued features); those cases need to be marked to opt out of the
-// check, plus min-value handling for numeric features.
+// carries some boolean features a covered GPU lacks (bug and hazard
+// workarounds); those cases need to be marked to opt out of the check.
static void
validateGenericFeatures(const Record *GPU,
- const DenseMap<const Record *, unsigned> &FeatureIdx) {
+ const DenseMap<const Record *, unsigned> &FeatureIdx,
+ const NumericPropertyMap &Properties) {
std::vector<const Record *> Covered =
GPU->getValueAsListOfDefs("CoveredGPUs");
if (Covered.empty())
return;
+ SetVector<const Record *> GenericClosure;
+ collectFeatureClosure(GPU, GenericClosure);
SetVector<const Record *> GenericFeatures =
- collectVisibleFeatures(GPU, FeatureIdx);
+ collectVisibleFeatures(GenericClosure.getArrayRef(), FeatureIdx);
+ SmallVector<int64_t> GenericValues;
+ for (const auto &[FieldName, Property] : Properties)
+ GenericValues.push_back(getFeatureValue(GPU, GenericClosure.getArrayRef(),
+ FieldName, Properties));
+
for (const Record *Member : Covered) {
+ SetVector<const Record *> MemberClosure;
+ collectFeatureClosure(Member, MemberClosure);
+ validateGenericNumericProperties(GPU, GenericValues, Member,
+ MemberClosure.getArrayRef(), Properties);
SetVector<const Record *> MemberFeatures =
- collectVisibleFeatures(Member, FeatureIdx);
+ collectVisibleFeatures(MemberClosure.getArrayRef(), FeatureIdx);
for (const Record *F : GenericFeatures) {
- if (!MemberFeatures.contains(F)) {
- PrintFatalError(GPU->getLoc(),
- "generic target '" + GPU->getValueAsString("Name") +
- "' exposes feature '" +
- F->getValueAsString("Name") +
- "' not supported by covered GPU '" +
- Member->getValueAsString("Name") + "'");
- }
+ if (Properties.contains(F->getValueAsString("FieldName")) ||
+ MemberFeatures.contains(F))
+ continue;
+
+ PrintFatalError(GPU->getLoc(),
+ "generic target '" + GPU->getValueAsString("Name") +
+ "' exposes feature '" + F->getValueAsString("Name") +
+ "' not supported by covered GPU '" +
+ Member->getValueAsString("Name") + "'");
}
}
}
-static void validateAMDGPU(const RecordKeeper &RK) {
+static void validateAMDGPU(const RecordKeeper &RK,
+ const NumericPropertyMap &Properties) {
DenseMap<const Record *, unsigned> FeatureIdx;
for (const auto &[Idx, F] :
enumerate(collectFrontendFeatures(RK, "AMDGPUFrontendVisibleFeatures")))
FeatureIdx[F] = Idx;
for (const Record *GPU : RK.getAllDerivedDefinitions("AMDGPUGPUInfo"))
- validateGenericFeatures(GPU, FeatureIdx);
+ validateGenericFeatures(GPU, FeatureIdx, Properties);
}
// Emit a GPU's feature bitset initializer: its feature closure intersected with
@@ -525,11 +653,8 @@ static void validateAMDGPU(const RecordKeeper &RK) {
// "AMDGPUFeatureBitset({FEAT_DPP, FEAT_CI_INSTS})".
static void
emitFeatureBitset(raw_ostream &OS, StringRef BitsetType, StringRef EnumPrefix,
- const Record *GPU,
+ ArrayRef<const Record *> Closure,
const DenseMap<const Record *, unsigned> &FeatureIdx) {
- SetVector<const Record *> Closure;
- collectFeatureClosure(GPU, Closure);
-
// Sort by bit index for stable output.
SmallVector<std::pair<unsigned, StringRef>> Bits;
for (const Record *F : Closure) {
@@ -548,44 +673,13 @@ emitFeatureBitset(raw_ostream &OS, StringRef BitsetType, StringRef EnumPrefix,
OS << "})";
}
-// The value of the SubtargetFeature in \p GPU's closure that sets \p FieldName,
-// or \p Default if it has none. Two features setting the same field to
-// different values is an error: SubtargetFeature silently takes the larger.
-static int64_t getFeatureValue(const Record *GPU, StringRef FieldName,
- int64_t Default) {
- SetVector<const Record *> Closure;
- collectFeatureClosure(GPU, Closure);
-
- const Record *Found = nullptr;
- int64_t Value = Default;
- for (const Record *F : Closure) {
- if (F->getValueAsString("FieldName") != FieldName)
- continue;
-
- int64_t V;
- if (!to_integer(F->getValueAsString("Value"), V)) {
- PrintFatalError(F->getLoc(), "feature '" + F->getValueAsString("Name") +
- "' must have an integer value");
- }
- if (Found && V != Value) {
- PrintFatalError(GPU->getLoc(),
- "GPU '" + GPU->getValueAsString("Name") +
- "' gets conflicting '" + FieldName +
- "' values from '" + Found->getValueAsString("Name") +
- "' and '" + F->getValueAsString("Name") + "'");
- }
- Found = F;
- Value = V;
- }
- return Value;
-}
-
/// Emit a GPUInfo table indexed by (GPUKind - AMDGPUFirstGPUKind). Name and
/// family strings are stored as offsets into the shared \p Names table.
static void
emitAMDGPUTable(raw_ostream &OS, const RecordKeeper &RK,
StringToOffsetTable &Names,
- const DenseMap<const Record *, unsigned> &FeatureIdx) {
+ const DenseMap<const Record *, unsigned> &FeatureIdx,
+ const NumericPropertyMap &Properties) {
std::vector<const Record *> Canon = collectAMDGPUCanonicals(RK);
if (Canon.empty())
return;
@@ -597,21 +691,28 @@ emitAMDGPUTable(raw_ostream &OS, const RecordKeeper &RK,
OS << ";\n"
"static constexpr GPUInfo AMDGPUGPUTable[] = {\n";
for (const Record *R : Canon) {
+ SetVector<const Record *> Closure;
+ collectFeatureClosure(R, Closure);
StringRef Name = R->getValueAsString("Name");
+ auto GetValue = [&](StringRef FieldName, int64_t Default) {
+ return getFeatureValue(R, Closure.getArrayRef(), FieldName, Properties,
+ Default);
+ };
OS << " {" << Names.GetOrAddStringOffset(Name) << ", ";
emitSubArch(OS, R);
OS << ", ";
- emitFeatureBitset(OS, "AMDGPUFeatureBitset", "FEAT_", R, FeatureIdx);
+ emitFeatureBitset(OS, "AMDGPUFeatureBitset", "FEAT_", Closure.getArrayRef(),
+ FeatureIdx);
OS << ", ";
emitIsaVersion(OS, R, '{', '}');
SmallString<16> Family;
raw_svector_ostream FamilyOS(Family);
emitArchFamily(FamilyOS, R);
OS << ", " << Names.GetOrAddStringOffset(Family) << ", "
- << getFeatureValue(R, "MaxWavesPerEU", 10) << ", "
- << getFeatureValue(R, "AddressableLocalMemorySize", 32768) << ", "
- << getFeatureValue(R, "LDSBankCount", 32) << ", "
- << getFeatureValue(R, "BufferResourceNumRecordsWidth", 0) << "},\n";
+ << GetValue("MaxWavesPerEU", 10) << ", "
+ << GetValue("AddressableLocalMemorySize", 32768) << ", "
+ << GetValue("LDSBankCount", 32) << ", "
+ << GetValue("BufferResourceNumRecordsWidth", 0) << "},\n";
}
OS << "};\n"
"#endif // GET_AMDGPU_GPU_TABLE\n\n";
@@ -756,7 +857,8 @@ static void emitAMDGPUSubArchNames(raw_ostream &OS, const RecordKeeper &RK,
}
static void emitAMDGPUTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
- validateAMDGPU(RK);
+ NumericPropertyMap Properties = collectNumericProperties(RK);
+ validateAMDGPU(RK, Properties);
OS << "// Autogenerated by AMDGPUTargetDefEmitter.cpp\n\n";
// R600.td and AMDGPU.td are separate top-level files, so a run sees exactly
@@ -810,7 +912,7 @@ static void emitAMDGPUTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
std::vector<unsigned> FeatureOffsets =
emitFeatureEnum(TablesOS, AMDGPUFeatureNaming, Features, Names);
- emitAMDGPUTable(TablesOS, RK, Names, FeatureIdx);
+ emitAMDGPUTable(TablesOS, RK, Names, FeatureIdx, Properties);
emitFeatureNames(TablesOS, AMDGPUFeatureNaming, FeatureOffsets);
emitAMDGPUAliases(TablesOS, RK, Names);
emitAMDGPUSubArchNames(TablesOS, RK, Names);
>From da93296daede918c463a53d05ea0bbf533344baa Mon Sep 17 00:00:00 2001
From: Chinmay Deshpande <chdeshpa at amd.com>
Date: Sat, 12 Sep 2026 18:11:08 -0700
Subject: [PATCH 2/2] Apply suggestion from @shiltian
Co-authored-by: Shilei Tian <i at tianshilei.me>
---
llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td b/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td
index 1f8719f46b362..ebb9fb6bfb798 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetParser.td
@@ -27,9 +27,7 @@ def AMDGPUGenericEqual : AMDGPUGenericNumericComparison;
//
// Fields without this metadata retain the exact feature-record matching used
// for frontend-visible capabilities.
-class AMDGPUGenericNumericProperty<
- string fieldName, int defaultValue,
- AMDGPUGenericNumericComparison comparison> {
+class AMDGPUGenericNumericProperty<string fieldName, int defaultValue, AMDGPUGenericNumericComparison comparison> {
string FieldName = fieldName;
int DefaultValue = defaultValue;
AMDGPUGenericNumericComparison GenericComparison = comparison;
More information about the llvm-commits
mailing list