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

[TableGen][GlobalISel] Use GIM_SwitchOpcode in Combiners #66864

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

Pierre-vh
Copy link
Contributor

The call to initOpcodeValuesMap was missing, causing the MatchTable to (unintentionally) not emit a SwitchMatcher. Also adds other code imported from GlobalISelEmitter.cpp to ensure rules are sorted by precedence as well.

Overall this improves GlobalISel compile-time performance by a noticeable amount. See #66751

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 20, 2023

@llvm/pr-subscribers-llvm-globalisel

Changes

The call to initOpcodeValuesMap was missing, causing the MatchTable to (unintentionally) not emit a SwitchMatcher. Also adds other code imported from GlobalISelEmitter.cpp to ensure rules are sorted by precedence as well.

Overall this improves GlobalISel compile-time performance by a noticeable amount. See #66751


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

1 Files Affected:

  • (modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+13)
diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index 08a4db2fb3dec91..b28915148ee5169 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -3652,6 +3652,9 @@ void GICombinerEmitter::gatherRules(
 }
 
 void GICombinerEmitter::run(raw_ostream &OS) {
+  InstructionOpcodeMatcher::initOpcodeValuesMap(Target);
+  LLTOperandMatcher::initTypeIDValuesMap();
+
   Records.startTimer("Gather rules");
   std::vector<RuleMatcher> Rules;
   gatherRules(Rules, Combiner->getValueAsListOfDefs("Rules"));
@@ -3666,6 +3669,16 @@ void GICombinerEmitter::run(raw_ostream &OS) {
   for (const auto &Rule : Rules)
     MaxTemporaries = std::max(MaxTemporaries, Rule.countRendererFns());
 
+  llvm::stable_sort(Rules, [&](const RuleMatcher &A, const RuleMatcher &B) {
+    if (A.isHigherPriorityThan(B)) {
+      assert(!B.isHigherPriorityThan(A) && "Cannot be more important "
+                                           "and less important at "
+                                           "the same time");
+      return true;
+    }
+    return false;
+  });
+
   const MatchTable Table = buildMatchTable(Rules);
 
   Records.startTimer("Emit combiner");

The call to `initOpcodeValuesMap` was missing, causing the MatchTable to never emit a `SwitchMatcher`.
Also adds other code imported from `GlobalISelEmitter.cpp` to ensure rules are sorted by precedence as well.

Overall this improves GlobalISel performance by a noticeable amount.
See llvm#66751
Copy link
Collaborator

@qcolombet qcolombet left a comment

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix, LGTM,

@aemerson aemerson linked an issue Sep 20, 2023 that may be closed by this pull request
@Pierre-vh Pierre-vh merged commit fcde8c8 into llvm:main Sep 20, 2023
2 of 3 checks passed
@Pierre-vh Pierre-vh deleted the fix-mt-perf branch September 20, 2023 07:32
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.

Large compile time regression with new MatchTable combiner
4 participants