Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV][TableGen] Generate RISCVTargetParserDef.inc from the new RISCVExtension tblgen information. #89335

Merged
merged 6 commits into from
Apr 23, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Apr 18, 2024

Instead of using RISCVISAInfo's extension information, use the extension found in tblgen after #89326.

We still need to use RISCVISAInfo code to get the sorting rules for the ISA string.

The ISA string we generate now is not quite the same extension we had before. No implied extensions are included in the generate string unless they are explicitly listed in RISCVProcessors.td. This primarily affects Zicsr being implied by F, V implying Zve*, and Zvlb implying a smaller Zvlb. All of these implication should be picked up when the string is used by the frontend.

The benefit is that we get a more manageable ISA string for humans to deal with.

This is a step towards generating RISCVISAInfo's extension list from tblgen.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Instead of using RISCVISAInfo's extension information, use the
extension found in tblgen after #89326.

We still need to use RISCVISAInfo code to get the sorting rules for
the ISA string.

The ISA string we generate now is not quite the same extension we
had before. No implied extensions are included in the generate string
unless they are explicitly listed in RISCVProcessors.td. This primarily
affects Zicsr being implied by F, V implying Zve*, and Zvlb implying a
smaller Zvl
b. All of these implication should be picked up when the
string is used by the frontend.

The benefit is that we get a more manageable ISA string for humans to
deal with.

This is a step towards generating RISCVISAInfo's extension list from
tblgen.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+395-382)
  • (modified) llvm/utils/TableGen/RISCVTargetDefEmitter.cpp (+23-20)
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 339c0397fa4518..b9034e7236b41b 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -10,115 +10,129 @@
 // RISC-V subtarget features and instruction predicates.
 //===----------------------------------------------------------------------===//
 
+class RISCVExtension<string n, int major, int minor, string f, string v,
+                     string d, list<RISCVExtension> i = []>
+  : SubtargetFeature<n, f, v, d,
+                     !foreach(feature, i, !cast<SubtargetFeature>(feature))> {
+  int MajorVersion = major;
+  int MinorVersion = minor;
+  bit Experimental = false;
+}
+
+class RISCVExperimentalExtension<string n, int major, int minor, string f,
+                                 string v, string d, list<RISCVExtension> i = []>
+  : RISCVExtension<"experimental-"#n, major, minor, f, v, d, i> {
+  let Experimental = true;
+}
+
 // Integer Extensions
 
 def FeatureStdExtI
-    : SubtargetFeature<"i", "HasStdExtI", "true",
-                       "'I' (Base Integer Instruction Set)">;
-
+    : RISCVExtension<"i", 2, 1, "HasStdExtI", "true",
+                     "'I' (Base Integer Instruction Set)">;
 def FeatureStdExtZic64b
-    : SubtargetFeature<"zic64b", "HasStdExtZic64b", "true",
-                       "'Zic64b' (Cache Block Size Is 64 Bytes)">;
+    : RISCVExtension<"zic64b", 1, 0, "HasStdExtZic64b", "true",
+                     "'Zic64b' (Cache Block Size Is 64 Bytes)">;
 
 def FeatureStdExtZicbom
-    : SubtargetFeature<"zicbom", "HasStdExtZicbom", "true",
-                       "'Zicbom' (Cache-Block Management Instructions)">;
+    : RISCVExtension<"zicbom", 1, 0, "HasStdExtZicbom", "true",
+                     "'Zicbom' (Cache-Block Management Instructions)">;
 def HasStdExtZicbom : Predicate<"Subtarget->hasStdExtZicbom()">,
                       AssemblerPredicate<(all_of FeatureStdExtZicbom),
                           "'Zicbom' (Cache-Block Management Instructions)">;
 
 def FeatureStdExtZicbop
-    : SubtargetFeature<"zicbop", "HasStdExtZicbop", "true",
-                       "'Zicbop' (Cache-Block Prefetch Instructions)">;
+    : RISCVExtension<"zicbop", 1, 0, "HasStdExtZicbop", "true",
+                     "'Zicbop' (Cache-Block Prefetch Instructions)">;
 def HasStdExtZicbop : Predicate<"Subtarget->hasStdExtZicbop()">,
                       AssemblerPredicate<(all_of FeatureStdExtZicbop),
                           "'Zicbop' (Cache-Block Prefetch Instructions)">;
 
 def FeatureStdExtZicboz
-    : SubtargetFeature<"zicboz", "HasStdExtZicboz", "true",
-                       "'Zicboz' (Cache-Block Zero Instructions)">;
+    : RISCVExtension<"zicboz", 1, 0, "HasStdExtZicboz", "true",
+                     "'Zicboz' (Cache-Block Zero Instructions)">;
 def HasStdExtZicboz : Predicate<"Subtarget->hasStdExtZicboz()">,
                       AssemblerPredicate<(all_of FeatureStdExtZicboz),
                           "'Zicboz' (Cache-Block Zero Instructions)">;
 
 def FeatureStdExtZiccamoa
-    : SubtargetFeature<"ziccamoa", "HasStdExtZiccamoa", "true",
-                       "'Ziccamoa' (Main Memory Supports All Atomics in A)">;
+    : RISCVExtension<"ziccamoa", 1, 0, "HasStdExtZiccamoa", "true",
+                     "'Ziccamoa' (Main Memory Supports All Atomics in A)">;
 
 def FeatureStdExtZiccif
-    : SubtargetFeature<"ziccif", "HasStdExtZiccif", "true",
-                       "'Ziccif' (Main Memory Supports Instruction Fetch with Atomicity Requirement)">;
+    : RISCVExtension<"ziccif", 1, 0, "HasStdExtZiccif", "true",
+                     "'Ziccif' (Main Memory Supports Instruction Fetch with Atomicity Requirement)">;
 
 def FeatureStdExtZicclsm
-    : SubtargetFeature<"zicclsm", "HasStdExtZicclsm", "true",
-                       "'Zicclsm' (Main Memory Supports Misaligned Loads/Stores)">;
+    : RISCVExtension<"zicclsm", 1, 0, "HasStdExtZicclsm", "true",
+                     "'Zicclsm' (Main Memory Supports Misaligned Loads/Stores)">;
 
 def FeatureStdExtZiccrse
-    : SubtargetFeature<"ziccrse", "HasStdExtZiccrse", "true",
-                       "'Ziccrse' (Main Memory Supports Forward Progress on LR/SC Sequences)">;
+    : RISCVExtension<"ziccrse", 1, 0, "HasStdExtZiccrse", "true",
+                     "'Ziccrse' (Main Memory Supports Forward Progress on LR/SC Sequences)">;
 
 def FeatureStdExtZicsr
-    : SubtargetFeature<"zicsr", "HasStdExtZicsr", "true",
-                       "'zicsr' (CSRs)">;
+    : RISCVExtension<"zicsr", 2, 0, "HasStdExtZicsr", "true",
+                     "'zicsr' (CSRs)">;
 def HasStdExtZicsr : Predicate<"Subtarget->hasStdExtZicsr()">,
                      AssemblerPredicate<(all_of FeatureStdExtZicsr),
                                         "'Zicsr' (CSRs)">;
 
 def FeatureStdExtZicntr
-    : SubtargetFeature<"zicntr", "HasStdExtZicntr", "true",
-                       "'Zicntr' (Base Counters and Timers)",
+    : RISCVExtension<"zicntr", 2, 0, "HasStdExtZicntr", "true",
+                     "'Zicntr' (Base Counters and Timers)",
                        [FeatureStdExtZicsr]>;
 
 def FeatureStdExtZicond
-    : SubtargetFeature<"zicond", "HasStdExtZicond", "true",
-                       "'Zicond' (Integer Conditional Operations)">;
+    : RISCVExtension<"zicond", 1, 0, "HasStdExtZicond", "true",
+                     "'Zicond' (Integer Conditional Operations)">;
 def HasStdExtZicond : Predicate<"Subtarget->hasStdExtZicond()">,
                       AssemblerPredicate<(all_of FeatureStdExtZicond),
                           "'Zicond' (Integer Conditional Operations)">;
 
 def FeatureStdExtZifencei
-    : SubtargetFeature<"zifencei", "HasStdExtZifencei", "true",
-                       "'Zifencei' (fence.i)">;
+    : RISCVExtension<"zifencei", 2, 0, "HasStdExtZifencei", "true",
+                     "'Zifencei' (fence.i)">;
 def HasStdExtZifencei : Predicate<"Subtarget->hasStdExtZifencei()">,
                         AssemblerPredicate<(all_of FeatureStdExtZifencei),
                                            "'Zifencei' (fence.i)">;
 
 def FeatureStdExtZihintpause
-    : SubtargetFeature<"zihintpause", "HasStdExtZihintpause", "true",
-                       "'Zihintpause' (Pause Hint)">;
+    : RISCVExtension<"zihintpause", 2, 0, "HasStdExtZihintpause", "true",
+                     "'Zihintpause' (Pause Hint)">;
 def HasStdExtZihintpause : Predicate<"Subtarget->hasStdExtZihintpause()">,
                            AssemblerPredicate<(all_of FeatureStdExtZihintpause),
                                               "'Zihintpause' (Pause Hint)">;
 
 def FeatureStdExtZihintntl
-    : SubtargetFeature<"zihintntl", "HasStdExtZihintntl", "true",
+    : RISCVExtension<"zihintntl", 1, 0, "HasStdExtZihintntl", "true",
                        "'Zihintntl' (Non-Temporal Locality Hints)">;
 def HasStdExtZihintntl : Predicate<"Subtarget->hasStdExtZihintntl()">,
                          AssemblerPredicate<(all_of FeatureStdExtZihintntl),
                              "'Zihintntl' (Non-Temporal Locality Hints)">;
 
 def FeatureStdExtZihpm
-    : SubtargetFeature<"zihpm", "HasStdExtZihpm", "true",
-                       "'Zihpm' (Hardware Performance Counters)",
-                       [FeatureStdExtZicsr]>;
+    : RISCVExtension<"zihpm", 2, 0, "HasStdExtZihpm", "true",
+                     "'Zihpm' (Hardware Performance Counters)",
+                     [FeatureStdExtZicsr]>;
 
-def FeatureStdExtZimop : SubtargetFeature<"zimop", "HasStdExtZimop", "true",
-                                          "'Zimop' (May-Be-Operations)">;
+def FeatureStdExtZimop : RISCVExtension<"zimop", 1, 0, "HasStdExtZimop", "true",
+                                        "'Zimop' (May-Be-Operations)">;
 def HasStdExtZimop : Predicate<"Subtarget->hasStdExtZimop()">,
                      AssemblerPredicate<(all_of FeatureStdExtZimop),
                                         "'Zimop' (May-Be-Operations)">;
 
 def FeatureStdExtZicfilp
-    : SubtargetFeature<"experimental-zicfilp", "HasStdExtZicfilp", "true",
-                       "'Zicfilp' (Landing pad)">;
+    : RISCVExperimentalExtension<"zicfilp", 0, 4, "HasStdExtZicfilp", "true",
+                                 "'Zicfilp' (Landing pad)">;
 def HasStdExtZicfilp : Predicate<"Subtarget->hasStdExtZicfilp()">,
                        AssemblerPredicate<(all_of FeatureStdExtZicfilp),
                                           "'Zicfilp' (Landing pad)">;
 
 def FeatureStdExtZicfiss
-    : SubtargetFeature<"experimental-zicfiss", "HasStdExtZicfiss", "true",
-                       "'Zicfiss' (Shadow stack)",
-                       [FeatureStdExtZicsr, FeatureStdExtZimop]>;
+    : RISCVExperimentalExtension<"zicfiss", 0, 4, "HasStdExtZicfiss", "true",
+                                 "'Zicfiss' (Shadow stack)",
+                                 [FeatureStdExtZicsr, FeatureStdExtZimop]>;
 def HasStdExtZicfiss : Predicate<"Subtarget->hasStdExtZicfiss()">,
                        AssemblerPredicate<(all_of FeatureStdExtZicfiss),
                                           "'Zicfiss' (Shadow stack)">;
@@ -127,15 +141,15 @@ def NoHasStdExtZicfiss : Predicate<"!Subtarget->hasStdExtZicfiss()">;
 // Multiply Extensions
 
 def FeatureStdExtM
-    : SubtargetFeature<"m", "HasStdExtM", "true",
-                       "'M' (Integer Multiplication and Division)">;
+    : RISCVExtension<"m", 2, 0, "HasStdExtM", "true",
+                     "'M' (Integer Multiplication and Division)">;
 def HasStdExtM : Predicate<"Subtarget->hasStdExtM()">,
                  AssemblerPredicate<(all_of FeatureStdExtM),
                      "'M' (Integer Multiplication and Division)">;
 
 def FeatureStdExtZmmul
-    : SubtargetFeature<"zmmul", "HasStdExtZmmul", "true",
-                       "'Zmmul' (Integer Multiplication)">;
+    : RISCVExtension<"zmmul", 1, 0, "HasStdExtZmmul", "true",
+                     "'Zmmul' (Integer Multiplication)">;
 
 def HasStdExtMOrZmmul
     : Predicate<"Subtarget->hasStdExtM() || Subtarget->hasStdExtZmmul()">,
@@ -146,28 +160,28 @@ def HasStdExtMOrZmmul
 // Atomic Extensions
 
 def FeatureStdExtA
-    : SubtargetFeature<"a", "HasStdExtA", "true",
-                       "'A' (Atomic Instructions)">;
+    : RISCVExtension<"a", 2, 1, "HasStdExtA", "true",
+                     "'A' (Atomic Instructions)">;
 def HasStdExtA : Predicate<"Subtarget->hasStdExtA()">,
                  AssemblerPredicate<(all_of FeatureStdExtA),
                                     "'A' (Atomic Instructions)">;
 
 def FeatureStdExtZtso
-    : SubtargetFeature<"experimental-ztso", "HasStdExtZtso", "true",
-                       "'Ztso' (Memory Model - Total Store Order)">;
+    : RISCVExperimentalExtension<"ztso", 0, 1, "HasStdExtZtso", "true",
+                                 "'Ztso' (Memory Model - Total Store Order)">;
 def HasStdExtZtso : Predicate<"Subtarget->hasStdExtZtso()">,
                     AssemblerPredicate<(all_of FeatureStdExtZtso),
                         "'Ztso' (Memory Model - Total Store Order)">;
 def NotHasStdExtZtso : Predicate<"!Subtarget->hasStdExtZtso()">;
 
-def FeatureStdExtZa64rs : SubtargetFeature<"za64rs", "HasStdExtZa64rs", "true",
-                                           "'Za64rs' (Reservation Set Size of at Most 64 Bytes)">;
+def FeatureStdExtZa64rs : RISCVExtension<"za64rs", 1, 0, "HasStdExtZa64rs", "true",
+                                         "'Za64rs' (Reservation Set Size of at Most 64 Bytes)">;
 
-def FeatureStdExtZa128rs : SubtargetFeature<"za128rs", "HasStdExtZa128rs", "true",
-                                            "'Za128rs' (Reservation Set Size of at Most 128 Bytes)">;
+def FeatureStdExtZa128rs : RISCVExtension<"za128rs", 1, 0, "HasStdExtZa128rs", "true",
+                                          "'Za128rs' (Reservation Set Size of at Most 128 Bytes)">;
 
 def FeatureStdExtZaamo
-    : SubtargetFeature<"experimental-zaamo", "HasStdExtZaamo", "true",
+    : RISCVExperimentalExtension<"zaamo", 0, 2, "HasStdExtZaamo", "true",
                        "'Zaamo' (Atomic Memory Operations)">;
 def HasStdExtAOrZaamo
     : Predicate<"Subtarget->hasStdExtA() || Subtarget->hasStdExtZaamo()">,
@@ -176,30 +190,30 @@ def HasStdExtAOrZaamo
                          "'Zaamo' (Atomic Memory Operations)">;
 
 def FeatureStdExtZabha
-    : SubtargetFeature<"experimental-zabha", "HasStdExtZabha", "true",
-                       "'Zabha' (Byte and Halfword Atomic Memory Operations)">;
+    : RISCVExperimentalExtension<"zabha", 1, 0, "HasStdExtZabha", "true",
+                                 "'Zabha' (Byte and Halfword Atomic Memory Operations)">;
 def HasStdExtZabha : Predicate<"Subtarget->hasStdExtZabha()">,
                      AssemblerPredicate<(all_of FeatureStdExtZabha),
                          "'Zabha' (Byte and Halfword Atomic Memory Operations)">;
 
 def FeatureStdExtZacas
-    : SubtargetFeature<"zacas", "HasStdExtZacas", "true",
-                       "'Zacas' (Atomic Compare-And-Swap Instructions)">;
+    : RISCVExtension<"zacas", 1, 0, "HasStdExtZacas", "true",
+                     "'Zacas' (Atomic Compare-And-Swap Instructions)">;
 def HasStdExtZacas : Predicate<"Subtarget->hasStdExtZacas()">,
                      AssemblerPredicate<(all_of FeatureStdExtZacas),
                          "'Zacas' (Atomic Compare-And-Swap Instructions)">;
 def NoStdExtZacas : Predicate<"!Subtarget->hasStdExtZacas()">;
 
 def FeatureStdExtZalasr
-    : SubtargetFeature<"experimental-zalasr", "HasStdExtZalasr", "true",
-                       "'Zalasr' (Load-Acquire and Store-Release Instructions)">;
+    : RISCVExperimentalExtension<"zalasr", 0, 1, "HasStdExtZalasr", "true",
+                                 "'Zalasr' (Load-Acquire and Store-Release Instructions)">;
 def HasStdExtZalasr : Predicate<"Subtarget->hasStdExtZalasr()">,
                       AssemblerPredicate<(all_of FeatureStdExtZalasr),
                           "'Zalasr' (Load-Acquire and Store-Release Instructions)">;
 
 def FeatureStdExtZalrsc
-    : SubtargetFeature<"experimental-zalrsc", "HasStdExtZalrsc", "true",
-                       "'Zalrsc' (Load-Reserved/Store-Conditional)">;
+    : RISCVExperimentalExtension<"zalrsc", 0, 2, "HasStdExtZalrsc", "true",
+                                 "'Zalrsc' (Load-Reserved/Store-Conditional)">;
 def HasStdExtAOrZalrsc
     : Predicate<"Subtarget->hasStdExtA() || Subtarget->hasStdExtZalrsc()">,
       AssemblerPredicate<(any_of FeatureStdExtA, FeatureStdExtZalrsc),
@@ -207,11 +221,11 @@ def HasStdExtAOrZalrsc
                          "'Zalrsc' (Load-Reserved/Store-Conditional)">;
 
 def FeatureStdExtZama16b
-    : SubtargetFeature<"zama16b", "HasStdExtZama16b", "true",
-                       "'Zama16b' (Atomic 16-byte misaligned loads, stores and AMOs)">;
+    : RISCVExtension<"zama16b", 1, 0, "HasStdExtZama16b", "true",
+                     "'Zama16b' (Atomic 16-byte misaligned loads, stores and AMOs)">;
 
-def FeatureStdExtZawrs : SubtargetFeature<"zawrs", "HasStdExtZawrs", "true",
-                                          "'Zawrs' (Wait on Reservation Set)">;
+def FeatureStdExtZawrs : RISCVExtension<"zawrs", 1, 0, "HasStdExtZawrs", "true",
+                                        "'Zawrs' (Wait on Reservation Set)">;
 def HasStdExtZawrs : Predicate<"Subtarget->hasStdExtZawrs()">,
                      AssemblerPredicate<(all_of FeatureStdExtZawrs),
                                         "'Zawrs' (Wait on Reservation Set)">;
@@ -219,43 +233,43 @@ def HasStdExtZawrs : Predicate<"Subtarget->hasStdExtZawrs()">,
 // Floating Point Extensions
 
 def FeatureStdExtF
-    : SubtargetFeature<"f", "HasStdExtF", "true",
-                       "'F' (Single-Precision Floating-Point)",
-                       [FeatureStdExtZicsr]>;
+    : RISCVExtension<"f", 2, 2, "HasStdExtF", "true",
+                     "'F' (Single-Precision Floating-Point)",
+                     [FeatureStdExtZicsr]>;
 def HasStdExtF : Predicate<"Subtarget->hasStdExtF()">,
                  AssemblerPredicate<(all_of FeatureStdExtF),
                                     "'F' (Single-Precision Floating-Point)">;
 
 def FeatureStdExtD
-    : SubtargetFeature<"d", "HasStdExtD", "true",
-                       "'D' (Double-Precision Floating-Point)",
-                       [FeatureStdExtF]>;
+    : RISCVExtension<"d", 2, 2, "HasStdExtD", "true",
+                     "'D' (Double-Precision Floating-Point)",
+                     [FeatureStdExtF]>;
 def HasStdExtD : Predicate<"Subtarget->hasStdExtD()">,
                  AssemblerPredicate<(all_of FeatureStdExtD),
                                     "'D' (Double-Precision Floating-Point)">;
 
 def FeatureStdExtZfhmin
-    : SubtargetFeature<"zfhmin", "HasStdExtZfhmin", "true",
-                       "'Zfhmin' (Half-Precision Floating-Point Minimal)",
-                       [FeatureStdExtF]>;
+    : RISCVExtension<"zfhmin", 1, 0, "HasStdExtZfhmin", "true",
+                     "'Zfhmin' (Half-Precision Floating-Point Minimal)",
+                     [FeatureStdExtF]>;
 def HasStdExtZfhmin : Predicate<"Subtarget->hasStdExtZfhmin()">,
                       AssemblerPredicate<(all_of FeatureStdExtZfhmin),
                           "'Zfh' (Half-Precision Floating-Point) or "
                           "'Zfhmin' (Half-Precision Floating-Point Minimal)">;
 
 def FeatureStdExtZfh
-    : SubtargetFeature<"zfh", "HasStdExtZfh", "true",
-                       "'Zfh' (Half-Precision Floating-Point)",
-                       [FeatureStdExtZfhmin]>;
+    : RISCVExtension<"zfh", 1, 0, "HasStdExtZfh", "true",
+                     "'Zfh' (Half-Precision Floating-Point)",
+                     [FeatureStdExtZfhmin]>;
 def HasStdExtZfh : Predicate<"Subtarget->hasStdExtZfh()">,
                    AssemblerPredicate<(all_of FeatureStdExtZfh),
                        "'Zfh' (Half-Precision Floating-Point)">;
 def NoStdExtZfh : Predicate<"!Subtarget->hasStdExtZfh()">;
 
 def FeatureStdExtZfbfmin
-    : SubtargetFeature<"experimental-zfbfmin", "HasStdExtZfbfmin", "true",
-                       "'Zfbfmin' (Scalar BF16 Converts)",
-                       [FeatureStdExtF]>;
+    : RISCVExperimentalExtension<"zfbfmin", 1, 0, "HasStdExtZfbfmin", "true",
+                                 "'Zfbfmin' (Scalar BF16 Converts)",
+                                 [FeatureStdExtF]>;
 def HasStdExtZfbfmin : Predicate<"Subtarget->hasStdExtZfbfmin()">,
                        AssemblerPredicate<(all_of FeatureStdExtZfbfmin),
                                           "'Zfbfmin' (Scalar BF16 Converts)">;
@@ -269,42 +283,42 @@ def HasHalfFPLoadStoreMove
                                     "'Zfbfmin' (Scalar BF16 Converts)">;
 
 def FeatureStdExtZfa
-    : SubtargetFeature<"zfa", "HasStdExtZfa", "true",
-                       "'Zfa' (Additional Floating-Point)",
-                       [FeatureStdExtF]>;
+    : RISCVExtension<"zfa", 1, 0, "HasStdExtZfa", "true",
+                     "'Zfa' (Additional Floating-Point)",
+                     [FeatureStdExtF]>;
 def HasStdExtZfa : Predicate<"Subtarget->hasStdExtZfa()">,
                    AssemblerPredicate<(all_of FeatureStdExtZfa),
                                       "'Zfa' (Additional Floating-Point)">;
 
 def FeatureStdExtZfinx
-    : SubtargetFeature<"zfinx", "HasStdExtZfinx", "true",
-                       "'Zfinx' (Float in Integer)",
-                       [FeatureStdExtZicsr]>;
+    : RISCVExtension<"zfinx", 1, 0, "HasStdExtZfinx", "true",
+                     "'Zfinx' (Float in Integer)",
+                     [FeatureStdExtZicsr]>;
 def HasStdExtZfinx : Predicate<"Subtarget->hasStdExtZfinx()">,
                      AssemblerPredicate<(all_of FeatureStdExtZfinx),
                                         "'Zfinx' (Float in Integer)">;
 
 def FeatureStdExtZdinx
-    : SubtargetFeature<"zdinx", "HasStdExtZdinx", "true",
-                       "'Zdinx' (Double in Integer)",
-                       [FeatureStdExtZfinx]>;
+    : RISCVExtension<"zdinx", 1, 0, "HasStdExtZdinx", "true",
+                     "'Zdinx' (Double in Integer)",
+                     [FeatureStdExtZfinx]>;
 def HasStdExtZdinx : Predicate<"Subtarget->hasStdExtZdinx()">,
                      AssemblerPredicate<(all_of FeatureStdExtZdinx),
                                         "'Zdinx' (Double in Int...
[truncated]

Copy link

github-actions bot commented Apr 19, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

std::vector<std::string> FeatureVector;
static void printMArch(raw_ostream &OS, const Record &Rec) {
std::map<std::string, std::pair<unsigned, unsigned>,
RISCVISAInfo::ExtensionComparator>
Copy link
Contributor

@wangpc-pp wangpc-pp Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should move ExtensionComparator to a suitable place so that we don't depend on RISCVISAInfo.h.
Edit: there is a cyclic dependency IIUC...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will eventually. I don't think there's a suitable place other than creating a new RISC-V specific file.

Copy link
Member

@fpetrogalli fpetrogalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for doing this @topperc !

On top of the few question I have, I think it would be great if you could provide a unit test that shows how some basic TD file is converted into the c++ code we need to include. Especially around march (whether it is manually specified or computed by the backend), and implicit feature that do not have an extension.

Thank you again!

Francesco

@@ -10,115 +10,129 @@
// RISC-V subtarget features and instruction predicates.
//===----------------------------------------------------------------------===//

class RISCVExtension<string n, int major, int minor, string f, string v,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please describe the parameters of the class.

@@ -10,115 +10,129 @@
// RISC-V subtarget features and instruction predicates.
//===----------------------------------------------------------------------===//

class RISCVExtension<string n, int major, int minor, string f, string v,
string d, list<RISCVExtension> i = []>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if an extension needs to depend from a SubtargetFeature that is not a RISCV extension?

llvm/utils/TableGen/RISCVTargetDefEmitter.cpp Show resolved Hide resolved
unsigned Major = Feature->getValueAsInt("MajorVersion");
unsigned Minor = Feature->getValueAsInt("MinorVersion");
Extensions.try_emplace(FeatureName.str(), Major, Minor);
} else if (FeatureName == "64bit")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sure I am missing something because this was here before... To me this looks like that on 64bit HM we do not load the extensions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're looping over the Features. This is just checking for one specific feature called "64bit". That feature isn't an "extension". It doesn't stop us from processing the other features.

@4vtomat
Copy link
Member

4vtomat commented Apr 20, 2024

In addition to extension lists, are we also going to auto generate implication lists in RISCVISAInfo.cpp?

@4vtomat
Copy link
Member

4vtomat commented Apr 20, 2024

One minor thing, the file generated is called RISCVTargetParserDef.inc lol~

@topperc
Copy link
Collaborator Author

topperc commented Apr 21, 2024

In addition to extension lists, are we also going to auto generate implication lists in RISCVISAInfo.cpp?

That's my plan

@topperc topperc changed the title [RISCV][TableGen] Generate RISCVTargetParser.inc from the new RISCVExtension tblgen information. [RISCV][TableGen] Generate RISCVTargetParserDef.inc from the new RISCVExtension tblgen information. Apr 21, 2024
…tension tblgen information.

Instead of using RISCVISAInfo's extension information, use the
extension found in tblgen after llvm#89326.

We still need to use RISCVISAInfo code to get the sorting rules for
the ISA string.

The ISA string we generate now is not quite the same extension we
had before. No implied extensions are included in the generate string
unless they are explicitly listed in RISCVProcessors.td. This primarily
affects Zicsr being implied by F, V implying Zve*, and Zvl*b implying a
smaller Zvl*b. All of these implication should be picked up when the
string is used by the frontend.

The benefit is that we get a more manageable ISA string for humans to
deal with.

This is a step towards generating RISCVISAInfo's extension list from
tblgen.
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@topperc topperc merged commit b64e483 into llvm:main Apr 23, 2024
3 of 4 checks passed
@topperc topperc deleted the pr/cpumarch-td branch April 23, 2024 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants