[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;
+
----------------
dsanders wrote:
> 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).
I decided to fix this by adding an LLTOperandMatcher in D29712


https://reviews.llvm.org/D29711