This is an archive of the discontinued LLVM Phabricator instance.

[GISel][Tablegen]: Make GlobalISelEmitter rule prioritization similar to that of selectionDAG
ClosedPublic

Authored by aditya_nandakumar on Feb 13 2018, 5:10 PM.

Details

Summary

This patch changes GlobalISelEmitter to rank patterns similar to how the DAG does it (ie it computes a score for a pattern and adds the added complexity to it).
This is so that the decision tree for GISelSelector remains compatible with that of SelectionDAG.

Diff Detail

Repository
rL LLVM

Event Timeline

volkan added a subscriber: volkan.Feb 14 2018, 11:07 AM
qcolombet accepted this revision.Feb 16 2018, 1:24 PM

Hi Aditya,

Given the initial goal of this backend is to provide a compatibility layer with SDISel, I think it just makes sense to reproduce the order we had in SDISel.

LGTM.

Cheers,
Q

This revision is now accepted and ready to land.Feb 16 2018, 1:24 PM
dsanders accepted this revision.Feb 16 2018, 1:40 PM

I can't say I'm keen on this since it's a warts-and-all port of the SelectionDAG implementation to GlobalISel but I do at least accept the argument that this is a compatibility layer. So long as as we keep it to the compatibility layer, I'm ok with it.

We still don't have a good solution for mixing GlobalISel rules and SelectionDAG rules if they're using different priority mechanisms but it seems reasonable to say that the requirement is that SelectionDAG rules should maintain the SelectionDAG priority order between themselves and GlobalISel rules may interleave with SelectionDAG rules in whatever way makes the most sense. One trivial answer is GlobalISel rules always have priority but it's probably better to merge the SelectionDAG rule list into the GlobalISel one using GlobalISel's priority ordering to find the appropriate insertion points. It's worth mentioning that we don't need to preserve SelectionDAG's order for mutually exclusive rules. It would therefore be good to expand on the mutually exclusive grouping mechanism and sort the resulting mini-lists rather than sorting the entire rule list first.

I can't say I'm keen on this since it's a warts-and-all port of the SelectionDAG implementation to GlobalISel but I do at least accept the argument that this is a compatibility layer. So long as as we keep it to the compatibility layer, I'm ok with it.

We still don't have a good solution for mixing GlobalISel rules and SelectionDAG rules if they're using different priority mechanisms but it seems reasonable to say that the requirement is that SelectionDAG rules should maintain the SelectionDAG priority order between themselves and GlobalISel rules may interleave with SelectionDAG rules in whatever way makes the most sense. One trivial answer is GlobalISel rules always have priority but it's probably better to merge the SelectionDAG rule list into the GlobalISel one using GlobalISel's priority ordering to find the appropriate insertion points. It's worth mentioning that we don't need to preserve SelectionDAG's order for mutually exclusive rules. It would therefore be good to expand on the mutually exclusive grouping mechanism and sort the resulting mini-lists rather than sorting the entire rule list first.

Thanks Daniel. That makes sense. We should revisit how to prioritize when we have a way of describing GISel only rules.

committed in r325401