[llvm] [TabeGen] Add `PreferSmallerInstructions` for Targets. (PR #83587)

Alfie Richards via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 7 07:09:32 PST 2024


https://github.com/AlfieRichardsArm updated https://github.com/llvm/llvm-project/pull/83587

>From 46bda4a6b93f3aec806ff6715d6969173cb17f06 Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Fri, 1 Mar 2024 14:42:43 +0000
Subject: [PATCH 1/4] [TabeGen] Add `PreferSmallerInstructions` for Targets.

This option means that in assembly matching instructions with smaller
encodings will be preferred.

This will be used for the ARM instruciton set where this is the correct
behaviour after some other refactoring.
---
 llvm/include/llvm/Target/Target.td        |  5 +++
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 38 +++++++++++++++++------
 llvm/utils/TableGen/CodeGenTarget.cpp     |  4 +++
 llvm/utils/TableGen/CodeGenTarget.h       |  5 +++
 4 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 0577c58f8da2df8..178a5489050cefd 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1743,6 +1743,11 @@ class Target {
   // setting hasExtraDefRegAllocReq and hasExtraSrcRegAllocReq to 1
   // for all opcodes if this flag is set to 0.
   int AllowRegisterRenaming = 0;
+
+  // PreferSmallerInstructions - Should the assembly matcher prefer the smaller
+  // instructions. 1 if the instruction set should sort by size,
+  // 0 otherwise.
+  bit PreferSmallerInstructions = 0;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index febd96086df27b1..6b32d19860d1493 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -502,6 +502,9 @@ struct MatchableInfo {
   /// matchable came from.
   Record *const TheDef;
 
+  // ResInstSize - The size of the resulting instruction for this matchable.
+  unsigned ResInstSize;
+
   /// DefRec - This is the definition that it came from.
   PointerUnion<const CodeGenInstruction *, const CodeGenInstAlias *> DefRec;
 
@@ -543,10 +546,12 @@ struct MatchableInfo {
 
   MatchableInfo(const CodeGenInstruction &CGI)
       : AsmVariantID(0), AsmString(CGI.AsmString), TheDef(CGI.TheDef),
-        DefRec(&CGI), UseInstAsmMatchConverter(true) {}
+        ResInstSize(TheDef->getValueAsInt("Size")), DefRec(&CGI),
+        UseInstAsmMatchConverter(true) {}
 
   MatchableInfo(std::unique_ptr<const CodeGenInstAlias> Alias)
       : AsmVariantID(0), AsmString(Alias->AsmString), TheDef(Alias->TheDef),
+        ResInstSize(Alias->ResultInst->TheDef->getValueAsInt("Size")),
         DefRec(Alias.release()), UseInstAsmMatchConverter(TheDef->getValueAsBit(
                                      "UseInstAsmMatchConverter")) {}
 
@@ -608,12 +613,18 @@ struct MatchableInfo {
   void buildInstructionResultOperands();
   void buildAliasResultOperands(bool AliasConstraintsAreChecked);
 
-  /// operator< - Compare two matchables.
-  bool operator<(const MatchableInfo &RHS) const {
+  /// shouldBeMatchedBefore - Compare two matchables for ordering.
+  bool shouldBeMatchedBefore(const MatchableInfo &RHS,
+                             const CodeGenTarget &Target) const {
     // The primary comparator is the instruction mnemonic.
     if (int Cmp = Mnemonic.compare_insensitive(RHS.Mnemonic))
       return Cmp == -1;
 
+    // Sort by the resultant instuctions size, eg. for ARM instructions
+    // we must choose the smallest matching instruction.
+    if (Target.getPreferSmallerInstructions() && ResInstSize != RHS.ResInstSize)
+      return ResInstSize < RHS.ResInstSize;
+
     if (AsmOperands.size() != RHS.AsmOperands.size())
       return AsmOperands.size() < RHS.AsmOperands.size();
 
@@ -652,7 +663,8 @@ struct MatchableInfo {
   /// couldMatchAmbiguouslyWith - Check whether this matchable could
   /// ambiguously match the same set of operands as \p RHS (without being a
   /// strictly superior match).
-  bool couldMatchAmbiguouslyWith(const MatchableInfo &RHS) const {
+  bool couldMatchAmbiguouslyWith(const MatchableInfo &RHS,
+                                 const CodeGenTarget &Target) const {
     // The primary comparator is the instruction mnemonic.
     if (Mnemonic != RHS.Mnemonic)
       return false;
@@ -661,6 +673,11 @@ struct MatchableInfo {
     if (AsmVariantID != RHS.AsmVariantID)
       return false;
 
+    // Sort by the resultant instuctions size, eg. for ARM instructions
+    // we must choose the smallest matching instruction.
+    if (Target.getPreferSmallerInstructions() && ResInstSize != RHS.ResInstSize)
+      return false;
+
     // The number of operands is unambiguous.
     if (AsmOperands.size() != RHS.AsmOperands.size())
       return false;
@@ -3224,17 +3241,18 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   // Sort the instruction table using the partial order on classes. We use
   // stable_sort to ensure that ambiguous instructions are still
   // deterministically ordered.
-  llvm::stable_sort(
-      Info.Matchables,
-      [](const std::unique_ptr<MatchableInfo> &a,
-         const std::unique_ptr<MatchableInfo> &b) { return *a < *b; });
+  llvm::stable_sort(Info.Matchables,
+                    [&Target](const std::unique_ptr<MatchableInfo> &a,
+                              const std::unique_ptr<MatchableInfo> &b) {
+                      return a->shouldBeMatchedBefore(*b, Target);
+                    });
 
 #ifdef EXPENSIVE_CHECKS
   // Verify that the table is sorted and operator < works transitively.
   for (auto I = Info.Matchables.begin(), E = Info.Matchables.end(); I != E;
        ++I) {
     for (auto J = I; J != E; ++J) {
-      assert(!(**J < **I));
+      assert(!((*J)->shouldBeMatchedBefore(**I, Target)));
     }
   }
 #endif
@@ -3253,7 +3271,7 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
         const MatchableInfo &A = **I;
         const MatchableInfo &B = **J;
 
-        if (A.couldMatchAmbiguouslyWith(B)) {
+        if (A.couldMatchAmbiguouslyWith(B, Target)) {
           errs() << "warning: ambiguous matchables:\n";
           A.dump();
           errs() << "\nis incomparable with:\n";
diff --git a/llvm/utils/TableGen/CodeGenTarget.cpp b/llvm/utils/TableGen/CodeGenTarget.cpp
index 980c9bdb6367f7b..9dea19188b6f0e3 100644
--- a/llvm/utils/TableGen/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/CodeGenTarget.cpp
@@ -332,6 +332,10 @@ bool CodeGenTarget::getAllowRegisterRenaming() const {
   return TargetRec->getValueAsInt("AllowRegisterRenaming");
 }
 
+bool CodeGenTarget::getPreferSmallerInstructions() const {
+  return TargetRec->getValueAsBit("PreferSmallerInstructions");
+}
+
 /// getAsmParser - Return the AssemblyParser definition for this target.
 ///
 Record *CodeGenTarget::getAsmParser() const {
diff --git a/llvm/utils/TableGen/CodeGenTarget.h b/llvm/utils/TableGen/CodeGenTarget.h
index 2ae3a3a2204dd03..36bebf0760a2b20 100644
--- a/llvm/utils/TableGen/CodeGenTarget.h
+++ b/llvm/utils/TableGen/CodeGenTarget.h
@@ -99,6 +99,11 @@ class CodeGenTarget {
   ///
   bool getAllowRegisterRenaming() const;
 
+  /// getPreferSmallerInstructions  - Return the PreferSmallerInstructions
+  /// flag value for this target.
+  ///
+  bool getPreferSmallerInstructions() const;
+
   /// getAsmParser - Return the AssemblyParser definition for this target.
   ///
   Record *getAsmParser() const;

>From c034c778c7b927b451ea53553b0d27db0809e5ed Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Tue, 5 Mar 2024 13:50:13 +0000
Subject: [PATCH 2/4] Move `PreferSmallerInstructions` to `AsmParser`

---
 llvm/include/llvm/Target/Target.td    | 9 ++++-----
 llvm/utils/TableGen/CodeGenTarget.cpp | 8 ++++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 178a5489050cefd..0206145e35ca4c4 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1564,6 +1564,10 @@ class AsmParser {
   // method shall be called for all operands as opposed to only those
   // that have their own specified custom parsers.
   bit CallCustomParserForAllOperands = false;
+
+  // PreferSmallerInstructions - Should the assembly matcher prefer the smaller
+  // instructions.
+  bit PreferSmallerInstructions = false;
 }
 def DefaultAsmParser : AsmParser;
 
@@ -1743,11 +1747,6 @@ class Target {
   // setting hasExtraDefRegAllocReq and hasExtraSrcRegAllocReq to 1
   // for all opcodes if this flag is set to 0.
   int AllowRegisterRenaming = 0;
-
-  // PreferSmallerInstructions - Should the assembly matcher prefer the smaller
-  // instructions. 1 if the instruction set should sort by size,
-  // 0 otherwise.
-  bit PreferSmallerInstructions = 0;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/utils/TableGen/CodeGenTarget.cpp b/llvm/utils/TableGen/CodeGenTarget.cpp
index 9dea19188b6f0e3..9cc703b31a74fe4 100644
--- a/llvm/utils/TableGen/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/CodeGenTarget.cpp
@@ -332,10 +332,6 @@ bool CodeGenTarget::getAllowRegisterRenaming() const {
   return TargetRec->getValueAsInt("AllowRegisterRenaming");
 }
 
-bool CodeGenTarget::getPreferSmallerInstructions() const {
-  return TargetRec->getValueAsBit("PreferSmallerInstructions");
-}
-
 /// getAsmParser - Return the AssemblyParser definition for this target.
 ///
 Record *CodeGenTarget::getAsmParser() const {
@@ -346,6 +342,10 @@ Record *CodeGenTarget::getAsmParser() const {
   return LI[AsmParserNum];
 }
 
+bool CodeGenTarget::getPreferSmallerInstructions() const {
+  return getAsmParser()->getValueAsBit("PreferSmallerInstructions");
+}
+
 /// getAsmParserVariant - Return the AssemblyParserVariant definition for
 /// this target.
 ///

>From b8dddb0455b6087f1bf2b2da36ff49f16351881d Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Wed, 6 Mar 2024 15:42:08 +0000
Subject: [PATCH 3/4] Addressing minor comments.

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 15 +++++++++------
 llvm/utils/TableGen/CodeGenTarget.cpp     |  4 ----
 llvm/utils/TableGen/CodeGenTarget.h       |  5 -----
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 6b32d19860d1493..3c1ed0e8e17bb21 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -375,6 +375,10 @@ class AsmVariantInfo {
   int AsmVariantNo;
 };
 
+bool getPreferSmallerInstructions(CodeGenTarget const &Target) {
+  return Target.getAsmParser()->getValueAsBit("PreferSmallerInstructions");
+}
+
 /// MatchableInfo - Helper class for storing the necessary information for an
 /// instruction or alias which is capable of being matched.
 struct MatchableInfo {
@@ -620,9 +624,9 @@ struct MatchableInfo {
     if (int Cmp = Mnemonic.compare_insensitive(RHS.Mnemonic))
       return Cmp == -1;
 
-    // Sort by the resultant instuctions size, eg. for ARM instructions
-    // we must choose the smallest matching instruction.
-    if (Target.getPreferSmallerInstructions() && ResInstSize != RHS.ResInstSize)
+    // (Optionally) Order by the resultant instuctions size.
+    // eg. for ARM thumb instructions smaller encodings should be preferred.
+    if (getPreferSmallerInstructions(Target) && ResInstSize != RHS.ResInstSize)
       return ResInstSize < RHS.ResInstSize;
 
     if (AsmOperands.size() != RHS.AsmOperands.size())
@@ -673,8 +677,7 @@ struct MatchableInfo {
     if (AsmVariantID != RHS.AsmVariantID)
       return false;
 
-    // Sort by the resultant instuctions size, eg. for ARM instructions
-    // we must choose the smallest matching instruction.
+    // The size of instruction is unambiguous.
     if (Target.getPreferSmallerInstructions() && ResInstSize != RHS.ResInstSize)
       return false;
 
@@ -3252,7 +3255,7 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   for (auto I = Info.Matchables.begin(), E = Info.Matchables.end(); I != E;
        ++I) {
     for (auto J = I; J != E; ++J) {
-      assert(!((*J)->shouldBeMatchedBefore(**I, Target)));
+      assert(!(*J)->shouldBeMatchedBefore(**I, Target));
     }
   }
 #endif
diff --git a/llvm/utils/TableGen/CodeGenTarget.cpp b/llvm/utils/TableGen/CodeGenTarget.cpp
index 9cc703b31a74fe4..980c9bdb6367f7b 100644
--- a/llvm/utils/TableGen/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/CodeGenTarget.cpp
@@ -342,10 +342,6 @@ Record *CodeGenTarget::getAsmParser() const {
   return LI[AsmParserNum];
 }
 
-bool CodeGenTarget::getPreferSmallerInstructions() const {
-  return getAsmParser()->getValueAsBit("PreferSmallerInstructions");
-}
-
 /// getAsmParserVariant - Return the AssemblyParserVariant definition for
 /// this target.
 ///
diff --git a/llvm/utils/TableGen/CodeGenTarget.h b/llvm/utils/TableGen/CodeGenTarget.h
index 36bebf0760a2b20..2ae3a3a2204dd03 100644
--- a/llvm/utils/TableGen/CodeGenTarget.h
+++ b/llvm/utils/TableGen/CodeGenTarget.h
@@ -99,11 +99,6 @@ class CodeGenTarget {
   ///
   bool getAllowRegisterRenaming() const;
 
-  /// getPreferSmallerInstructions  - Return the PreferSmallerInstructions
-  /// flag value for this target.
-  ///
-  bool getPreferSmallerInstructions() const;
-
   /// getAsmParser - Return the AssemblyParser definition for this target.
   ///
   Record *getAsmParser() const;

>From 7c3b6e1f467a1ed9a0e4201cbf20dfdf7557e699 Mon Sep 17 00:00:00 2001
From: Alfie Richards <156316945+AlfieRichardsArm at users.noreply.github.com>
Date: Wed, 6 Mar 2024 19:10:38 +0000
Subject: [PATCH 4/4] Update AsmMatcherEmitter.cpp

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 3c1ed0e8e17bb21..a6ff6e8e7c00b02 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -678,7 +678,7 @@ struct MatchableInfo {
       return false;
 
     // The size of instruction is unambiguous.
-    if (Target.getPreferSmallerInstructions() && ResInstSize != RHS.ResInstSize)
+    if (getPreferSmallerInstructions(Target) && ResInstSize != RHS.ResInstSize)
       return false;
 
     // The number of operands is unambiguous.



More information about the llvm-commits mailing list