Page MenuHomePhabricator

[gicombiner] Hoist pure C++ combine into the tablegen definition

Authored by dsanders on Oct 3 2019, 2:41 PM.



This is just moving the existing C++ code around and will be NFC w.r.t
AArch64. Renamed 'CombineBr' to something more descriptive
('ElideByByInvertingCond') at the same time.

The remaining combines in AArch64PreLegalizeCombiner require features that
aren't implemented at this point and will be hoisted as they are added.

Depends on D68424

Diff Detail

Event Timeline

dsanders created this revision.Oct 3 2019, 2:41 PM
volkan added inline comments.Oct 16 2019, 1:22 PM

Rules must have a matcher, right? If so, why do we need 1 && here?


Don't we need to get the Matcher here as we do that for Applyer below?

dsanders marked 2 inline comments as done.Oct 16 2019, 3:29 PM
dsanders added inline comments.

Yes but they don't necessarily use the trap-door into the C++ block (eventually none will and I'll remove it). At this point in the patch series, that trap-door is the only thing the matcher can use so they all use it but in subsequent patches there will be enough features to move the code into DAG matchers and/or custom predicates. Even later, predicates that don't get tested as part of the decision tree will be resolved in this if-statement.

For why the 1 && is in the output, it's just to keep the code simple. Without it, I'd have to special case the case where there's no checks but with it I can just have each check emit && foo()


At the moment, the matcher can only contain a code block and this is essentially a trap-door that allows me to fix up the generated code and keep things working while the features are going in. I could rename getCode() to getMatcherFixupCode() if that helps.

dsanders updated this revision to Diff 225325.Oct 16 2019, 3:51 PM

Rename the variable holding the fixup code block

volkan accepted this revision.Oct 16 2019, 3:59 PM

Thanks for explaining, LGTM.


I think that would make it easier to understand.

This revision is now accepted and ready to land.Oct 16 2019, 3:59 PM
This revision was automatically updated to reflect the committed changes.