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

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar added inline comments.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:536
+  /// Returns true if this object is more important than B.
+  bool isHigherPriorityThan(const RuleMatcher &B) const {
+    // Rules involving more match roots have higher priority.
----------------
aditya_nandakumar wrote:
> ab wrote:
> > This should maybe take into account the AddedComplexity somehow, but that's not really compatible with the isHigherPriorityThan scheme :/
> > 
> > I'm not sure using AddedComplexity is a good idea in the first place: it's very magic-number-y and totally SDAG implementation specific.  Still, care has been put into that, so if there's any value we can easily extract from that, let's do it.
> > 
> > Maybe instead sum up the "complexity" of each Matcher?  We'd want to make sure to prioritize significant metrics like more instructions over operand kinds and such.
> > 
> > Assuming you don't have another usage in mind for the *Kind enums, that might let you simplify that to virtual methods in each Matcher returning its cost.
> Our backend has scripts that takes into account ISA information and auto generates various DAGPatterns and also calculates the added complexity for each pattern and emits them (not in sorted order by complexity).
> If we try to reprioritize patterns and come up with newer metrics/aggregates for matching, it may not be accurate/expected (wrt to pattern authors/scripts knowing about costs).
> 
Perhaps there needs to be a "-respect-added-complexity" option so backends can rely on GlobalISel not re-ordering patterns it thinks is better?


https://reviews.llvm.org/D29711