[llvm] [TableGen] Add `PreferSmallerInstructions` for Targets. (PR #83587)
Alfie Richards via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 13 17:11:41 PDT 2024
https://github.com/AlfieRichardsArm updated https://github.com/llvm/llvm-project/pull/83587
>From 36edb74260a6d9bed41b2979bc8eeac1b9cf7912 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 0577c58f8da2df..178a5489050cef 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 febd96086df27b..6b32d19860d149 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 e1cf33e7f62ffc..b46378428b2717 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 e109c717dc018e..c86c81c3494146 100644
--- a/llvm/utils/TableGen/CodeGenTarget.h
+++ b/llvm/utils/TableGen/CodeGenTarget.h
@@ -100,6 +100,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 f11805bf66af4f3aea76a2b3b38726d3ace368b8 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 178a5489050cef..0206145e35ca4c 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 b46378428b2717..d98ec765a54262 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 4aa00ab76915a72f02a4e9028febf7e24ff5c06d 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 6b32d19860d149..3c1ed0e8e17bb2 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 d98ec765a54262..e1cf33e7f62ffc 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 c86c81c3494146..e109c717dc018e 100644
--- a/llvm/utils/TableGen/CodeGenTarget.h
+++ b/llvm/utils/TableGen/CodeGenTarget.h
@@ -100,11 +100,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 1cee0ef0220174c0447e0b6713d0309dab6a39f5 Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Wed, 13 Mar 2024 16:47:11 +0000
Subject: [PATCH 4/4] Fix removing UB for ResInstSize initialisation
---
llvm/utils/TableGen/AsmMatcherEmitter.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 3c1ed0e8e17bb2..b440f60abf5aba 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -564,9 +564,9 @@ struct MatchableInfo {
// where it was copied while being in an owning state.
MatchableInfo(const MatchableInfo &RHS)
: AsmVariantID(RHS.AsmVariantID), AsmString(RHS.AsmString),
- TheDef(RHS.TheDef), DefRec(RHS.DefRec), ResOperands(RHS.ResOperands),
- Mnemonic(RHS.Mnemonic), AsmOperands(RHS.AsmOperands),
- RequiredFeatures(RHS.RequiredFeatures),
+ TheDef(RHS.TheDef), ResInstSize(RHS.ResInstSize), DefRec(RHS.DefRec),
+ ResOperands(RHS.ResOperands), Mnemonic(RHS.Mnemonic),
+ AsmOperands(RHS.AsmOperands), RequiredFeatures(RHS.RequiredFeatures),
ConversionFnKind(RHS.ConversionFnKind),
HasDeprecation(RHS.HasDeprecation),
UseInstAsmMatchConverter(RHS.UseInstAsmMatchConverter) {
@@ -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