Patch adds two new GICombinerRules for G_SELECT. The rules include: combining selects with undef comparisons into their first selectee value, and to combine away selects with constant comparisons. Patch additionally adds a new combiner test for the AArch64 target to test these new G_SELECT combiner rules and the existing select_same_val combiner rule.
This may not be correct? See 1cf6f210a2ed87dcda2183fffd6f9aa17b5c493c
Looks like my local build wasn't building all the targets (sorry, fixed now). The AMDGPU test failed as a result of moving the existing select_same_val combiner out of the identity_combines group. The diff update moves select_same_val back to identity_combines group. The update does not address @arsenm's comment (waiting on feedback from @craig.topper).
I think all you should need is an equivalent to the NotPoison check in 1cf6f210a2ed87dcda2183fffd6f9aa17b5c493c, plus appropriate testcases.
Maybe? It's unclear to me what makes a ConstantExpr poison in the first place, and how combining with it would result in incorrect code (...new hire rolling through).
This matches InstSimplify, although I do wonder why the choice isn't to select the RHS in this case. Since we generally prefer to canonicalize lower complexity expressions to the RHS, it seems like that would generally be the cheaper choice. I do think this should match the InstSimplify behavior though
Probably should split this into its own patch since it's questionable?
I've been thinking GIMatchData has the annoying property where every combine that uses the same thing needs to define its own GIDefMatchData.
Sounds reasonable. Will split that combine off into another patch.
Removed the combine for undef selectee value. Will open a separate patch to track that work.
I'm thinking there's no reason this needs to distinguish the undef and constant cases with D84165. However, that's making me second guess whether accepting G_IMPLICIT_DEF as an ordinary constant is a good idea if it can trigger the potentially poison case
Thanks for the reviews. I don't have commit access, would be appreciated if someone could commit on my behalf. Thanks!
- Rebased and resolved conflicts
- Fixed test/CodeGen/AMDGPU/GlobalISel/postlegalizercombiner-select.mir to continue testing unmerge with same regs
It was conflicted until the rebase update, so I doubt it was committed. Thanks for committing this!