[PATCH] D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 24 15:17:19 PDT 2017


qcolombet added a comment.

> For this, I was thinking there would be two kinds of partitioner. Those that deal with structure (number of operands, opcodes?*, nested instructions, etc.) and those that deal with predicates.

I would rather avoid the partitioners to be kind of "semantic-aware". If at all possible I would rather stick to simple concept like put "actions" that are identical together. On a different topic, the same apply for the matcher. Right now, we do the distinction between capturing, matching and so on, but all in all, this is just actions.

What I am saying is the design is more complicated than I would have thought and I wonder if on the long run it won't get in the way of modifying it... Unless it is well documented, in particular the intent :).



================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:594-597
+  void clearImplicitMap() {
+    NextInsnVarID = 0;
+    InsnVariableIDs.clear();
+  };
----------------
dsanders wrote:
> I don't understand why this is needed.
Admittedly, this is a hack around the fact that capturing a variable is not an action, but instead something that happen somewhat magically when we need to access something (defineInsnVar).

Now, how does this work:
1. The "predicates" are self contained now, i.e., they reference all the InsnID and OpIdx they are going to access. In particular, they don't rely on a Rule at emission time to get this information (though the information needs to be in sync for this to work, thus the bunch of asserts added) Thanks to that, we can check that two predicates are really identical (as opposed to checking the same thing, but on different variables).
2. When building the predicates we implicitly defines all the variables that are going to be accessed in Rule.
3. We clean the implicit def in Rule (this method)
4. When we emit Rule we check that the defined variables are equal to what we chose at build time for the predicate

If we skip #3, Rule is going to keep increasing the InsnVarID and the numbers we came up at build time won't reference the right thing.

Does it make sense? (look line 2842 for the call to that method)


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:3053-3062
+  std::stable_sort(Rules.begin(), Rules.end(), [&](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;
----------------
dsanders wrote:
> This bit is correctness rather than optimization
You're right, this is supposed to happen before the call to OptimizeRules.
Looks like I screwed up my squash :P.
I'll update the patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D39034





More information about the llvm-commits mailing list