[PATCH] D29711: [globalisel] Sort RuleMatchers by priority.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:299
+    if (predicates_size() > B.predicates_size())
+      return true;
+
----------------
rovka wrote:
> dsanders wrote:
> > rovka wrote:
> > > Shouldn't you also return false if predicates_size() < B.predicates_size() ?
> > I think that makes sense but I don't think it has a functional effect on the matcher. I'll make this change (and the others like it).
> It might have a functional effect on the sort, but I'll admit I haven't thought enough about it to be sure. Anyway, it looks like a simple way to make sure we're preserving asymmetry at least in a few extra cases.
While updating the next patch, I've spotted a bug that the previous version of this patch was hiding.

We shouldn't be checking the size of the predicates list first because IntOperandMatcher needs priority over the RegisterBankOperandMatcher+LLTOperandMatcher pair used by register operands.

I'll post an update that fixes this tomorrow (either by adding an LLTOperandMatcher, or moving the predicate list size check down).


https://reviews.llvm.org/D29711