[TableGen] Print a warning when a Processor contains duplicate Features / TuneFeatures #137864

mshockwave · 2025-04-29T19:11:18Z

I don't think a processor should normally contains duplicate features. I'm open to make this an error rather than a warning.

…es / TuneFeatures

llvmbot · 2025-04-29T19:11:54Z

@llvm/pr-subscribers-tablegen

Author: Min-Yih Hsu (mshockwave)

Changes

I don't think a processor should normally contains duplicate features. I'm open to make this an error rather than a warning.

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

2 Files Affected:

(added) llvm/test/TableGen/DuplicateProcessorFeatures.td (+16)
(modified) llvm/utils/TableGen/SubtargetEmitter.cpp (+12)

diff --git a/llvm/test/TableGen/DuplicateProcessorFeatures.td b/llvm/test/TableGen/DuplicateProcessorFeatures.td
new file mode 100644
index 0000000000000..324a70f2a9d9e
--- /dev/null
+++ b/llvm/test/TableGen/DuplicateProcessorFeatures.td
@@ -0,0 +1,16 @@
+// RUN: llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+
+def FeatureA : SubtargetFeature<"NameA", "", "", "">;
+def FeatureB : SubtargetFeature<"NameB", "", "", "">;
+def FeatureC : SubtargetFeature<"NameC", "", "", "">;
+
+// CHECK: warning: Processor CPU0 has duplicate Features: NameA
+def P0 : ProcessorModel<"CPU0", NoSchedModel, [FeatureA, FeatureB, FeatureA]>;
+// CHECK: warning: Processor CPU1 has duplicate TuneFeatures: NameB
+def P1 : ProcessorModel<"CPU1", NoSchedModel,
+                        /*Features=*/[FeatureA, FeatureB, FeatureC],
+                        /*TuneFeatures=*/[FeatureB, FeatureC, FeatureB]>;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 8d2fc99f84670..c608e85cddffa 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -333,6 +333,16 @@ unsigned SubtargetEmitter::cpuNames(raw_ostream &OS) {
   return Names.size();
 }
 
+static void checkDuplicateCPUFeatures(StringRef CPUName, StringRef FeatureKind,
+                                      ConstRecVec Features) {
+  SmallPtrSet<const Record *, 8> FeatureSet;
+  for (const auto *FeatureRec : Features) {
+    if (!FeatureSet.insert(FeatureRec).second)
+      PrintWarning("Processor " + CPUName + " has duplicate " + FeatureKind +
+                   ": " + FeatureRec->getValueAsString("Name"));
+  }
+}
+
 //
 // CPUKeyValues - Emit data of all the subtarget processors.  Used by command
 // line.
@@ -357,8 +367,10 @@ unsigned SubtargetEmitter::cpuKeyValues(raw_ostream &OS,
   for (const Record *Processor : ProcessorList) {
     StringRef Name = Processor->getValueAsString("Name");
     ConstRecVec FeatureList = Processor->getValueAsListOfDefs("Features");
+    checkDuplicateCPUFeatures(Name, "Features", FeatureList);
     ConstRecVec TuneFeatureList =
         Processor->getValueAsListOfDefs("TuneFeatures");
+    checkDuplicateCPUFeatures(Name, "TuneFeatures", TuneFeatureList);
 
     // Emit as "{ "cpu", "description", 0, { f1 , f2 , ... fn } },".
     OS << " { "

llvm/utils/TableGen/SubtargetEmitter.cpp

jurahul · 2025-04-29T21:26:47Z

llvm/utils/TableGen/SubtargetEmitter.cpp

+      PrintWarning("Processor " + CPUName +
+                   " contains duplicate tune feature '" +
+                   TuneFeatureRec->getValueAsString("Name") + "'");
+    if (FeatureSet.count(TuneFeatureRec))


nit: FeatureSet.contains() for a more canonical form?

jurahul · 2025-04-29T21:29:01Z

LGTM with a small nit.

topperc · 2025-04-29T21:34:59Z

llvm/utils/TableGen/SubtargetEmitter.cpp

+static void checkDuplicateCPUFeatures(StringRef CPUName,
+                                      ArrayRef<const Record *> Features,
+                                      ArrayRef<const Record *> TuneFeatures) {
+  // We had made sure each SubtargetFeature Record has a unique name, so we can


llvm-ci · 2025-04-30T01:59:32Z

LLVM Buildbot has detected a new failure on builder bolt-x86_64-ubuntu-nfc running on bolt-worker while building llvm at step 9 "test-build-bolt-check-large-bolt".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/92/builds/17955

Here is the relevant piece of the build log for the reference

Step 9 (test-build-bolt-check-large-bolt) failure: 1200 seconds without output running [b'ninja', b'check-large-bolt'], attempting to kill
...
PASS: bolt-tests :: X86/exceptions-split-strip.test (6 of 16)
PASS: bolt-tests :: X86/openssl.test (7 of 16)
XFAIL: bolt-tests :: AArch64/bzip2-nolbr.test (8 of 16)
PASS: bolt-tests :: X86/libmysqlclient-entry-cold.test (9 of 16)
PASS: bolt-tests :: AArch64/bzip2.test (10 of 16)
PASS: bolt-tests :: X86/cond-tail.test (11 of 16)
PASS: bolt-tests :: X86/debugTypesBug.test (12 of 16)
PASS: bolt-tests :: X86/R_X86_64_64.pic.test (13 of 16)
PASS: bolt-tests :: X86/clang.test (14 of 16)
PASS: bolt-tests :: X86/clang7-pic.test (15 of 16)
command timed out: 1200 seconds without output running [b'ninja', b'check-large-bolt'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1268.406105

jayfoad · 2025-04-30T12:00:20Z

This is printing new warnings when I build llc:

[1938/2915] Building PPCGenSubtargetInfo.inc...
warning: Processor future contains duplicate feature 'predictable-select-expensive'
warning: Processor pwr10 contains duplicate feature 'predictable-select-expensive'
warning: Processor pwr11 contains duplicate feature 'predictable-select-expensive'
warning: Processor pwr9 contains duplicate feature 'predictable-select-expensive'

mshockwave · 2025-04-30T18:26:07Z

This is printing new warnings when I build llc:

[1938/2915] Building PPCGenSubtargetInfo.inc...
warning: Processor future contains duplicate feature 'predictable-select-expensive'
warning: Processor pwr10 contains duplicate feature 'predictable-select-expensive'
warning: Processor pwr11 contains duplicate feature 'predictable-select-expensive'
warning: Processor pwr9 contains duplicate feature 'predictable-select-expensive'

Pending #137903

jurahul · 2025-05-02T12:28:51Z

I also see:

[2704/5914] Building CSKYGenSubtargetInfo.inc...
warning: Processor ck807e contains duplicate feature 'edsp'
warning: Processor ck807e contains duplicate feature 'dsp1e2'
warning: Processor ck807e contains duplicate feature 'dspe60'
warning: Processor ck807ef contains duplicate feature 'edsp'
warning: Processor ck807ef contains duplicate feature 'dsp1e2'
warning: Processor ck807ef contains duplicate feature 'dspe60'

mshockwave · 2025-05-02T18:03:02Z

I also see:

[2704/5914] Building CSKYGenSubtargetInfo.inc... warning: Processor ck807e contains duplicate feature 'edsp' warning: Processor ck807e contains duplicate feature 'dsp1e2' warning: Processor ck807e contains duplicate feature 'dspe60' warning: Processor ck807ef contains duplicate feature 'edsp' warning: Processor ck807ef contains duplicate feature 'dsp1e2' warning: Processor ck807ef contains duplicate feature 'dspe60'

thanks for catching that. PR: #138316

…es / TuneFeatures (llvm#137864) I don't think a processor should normally contains duplicate features. This patch prints a warning whenever that happens in either the features or tune features.

[TableGen] Print a warning when a Processor contains duplicate Featur…

a5144cc

…es / TuneFeatures

mshockwave requested review from jroelofs and jurahul April 29, 2025 19:11

llvmbot added the tablegen label Apr 29, 2025

This was referenced Apr 29, 2025

[RISCV][NFC] Remove duplicate extensions from tt-ascalon-d8 CPU #137865

Merged

[RISCV] Add processor definition for SiFive P870 #137725

Merged

jurahul reviewed Apr 29, 2025

View reviewed changes

llvm/utils/TableGen/SubtargetEmitter.cpp Outdated Show resolved Hide resolved

jurahul reviewed Apr 29, 2025

View reviewed changes

llvm/utils/TableGen/SubtargetEmitter.cpp Outdated Show resolved Hide resolved

jroelofs requested a review from ahmedbougacha April 29, 2025 20:21

fixup! Address review comments

2575a09

jurahul reviewed Apr 29, 2025

View reviewed changes

jurahul self-requested a review April 29, 2025 21:29

jurahul approved these changes Apr 29, 2025

View reviewed changes

topperc reviewed Apr 29, 2025

View reviewed changes

fixup! Address review comments

49ab7f3

mshockwave merged commit 951292b into llvm:main Apr 29, 2025
6 of 11 checks passed

mshockwave deleted the patch/tblgen/duplicate-feature-detector branch April 29, 2025 23:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TableGen] Print a warning when a Processor contains duplicate Features / TuneFeatures #137864

[TableGen] Print a warning when a Processor contains duplicate Features / TuneFeatures #137864

Uh oh!

mshockwave commented Apr 29, 2025

Uh oh!

llvmbot commented Apr 29, 2025

Uh oh!

Uh oh!

Uh oh!

jurahul Apr 29, 2025 •

edited

Loading

Uh oh!

mshockwave Apr 29, 2025

Uh oh!

jurahul commented Apr 29, 2025

Uh oh!

topperc Apr 29, 2025

Uh oh!

mshockwave Apr 29, 2025

Uh oh!

Uh oh!

llvm-ci commented Apr 30, 2025

Uh oh!

jayfoad commented Apr 30, 2025

Uh oh!

mshockwave commented Apr 30, 2025

Uh oh!

jurahul commented May 2, 2025

Uh oh!

mshockwave commented May 2, 2025

Uh oh!

Uh oh!

[TableGen] Print a warning when a Processor contains duplicate Features / TuneFeatures #137864

[TableGen] Print a warning when a Processor contains duplicate Features / TuneFeatures #137864

Uh oh!

Conversation

mshockwave commented Apr 29, 2025

Uh oh!

llvmbot commented Apr 29, 2025

Uh oh!

Uh oh!

Uh oh!

jurahul Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshockwave Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

jurahul commented Apr 29, 2025

Uh oh!

topperc Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

mshockwave Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Apr 30, 2025

Uh oh!

jayfoad commented Apr 30, 2025

Uh oh!

mshockwave commented Apr 30, 2025

Uh oh!

jurahul commented May 2, 2025

Uh oh!

mshockwave commented May 2, 2025

Uh oh!

Uh oh!

jurahul Apr 29, 2025 •

edited

Loading