[RFC] MatchTable-based GlobalISel Combiners

[Pierre-vh]

Hi, sorry for the late reply - I somehow didn’t get an email notification and missed this.

@dsanders

The one concern I have is whether I’ll still be able to do things like the sextload/zextload combine in this implementation. The background to this is that DAGISel would only consider one use at a time and this necessitated excessive use of hasOneUse() (or the hasOneNonDbgUse() which isn’t sensitive to -g) in our downstream target to prevent combines applied in the context of one use leading to worse code overall across multiple uses.

I’m not sure I understand, can you write a small example? No upstream combines were affected by this change, so I assume it’s a downstream issue?

In any case, I think the MatchTable is very flexible. It’s simply a framework for a state machine after all, so given a few refactors I’m sure we can make almost anything work. If I understand correctly, you would want a system to make a MIR pattern only match if all the instructions can be folded out? That’s definitely possible I believe.

@dsanders

Mid to long term no but short term we could use a bit of time. We’re currently playing catchup on several fronts at the moment and have quite a lot of things to un-revert on our upstream-tracking branch. In particular, opaque pointers and the removal of the legacy pass manager. It would be difficult to take on a third right now. That said, I don’t really want to be in your way. If you can give us a couple months before things start being removed that would help us out a fair bit. If not, that’s fine too, it’s just the cost of being behind .

I don’t mind waiting a bit if needed, I was planning to deprecate the backend this week & remove it sometime by the end of summer (probably late August/early September). I also plan do to a few refactors while I’m at it to make the Combiner pass work more like the ISel pass (= don’t make it re-instantiate the implementation on every instruction). Would that work for you?

If you encounter any major issues while porting this feel free to open an issue on GitHub and tag me, or simply send me an email directly. I’m happy to help!

@dsanders

If I’m reading this RFC correctly it’s not really going as far as your original position of re-using all the DAGISel tablegen syntax. The DAGISel nodes really didn’t fit GlobalISel which is why the ISel support still has holes and we end up using C++ (in our downstream target a lot).

Note that I’m also working on a MIR pattern system based on this RFC (re-using the same syntax as current MIR patterns, just with a lot more features). My goal is make it possible to rewrite most combines as pure tablegen/MIR patterns. That’s a separate topic though and we can discuss it when/if I publish an RFC for it (it’d be a very big patch so it needs to cook a bit longer)