This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Add basic predicated add/sub patterns
AbandonedPublic

Authored by dmgreen on Apr 26 2023, 10:01 AM.

Details

Summary

This adds the most basic form of predicated add and sub patterns, selecting from vselect(m, add(x, y), x).

Diff Detail

Event Timeline

dmgreen created this revision.Apr 26 2023, 10:01 AM
dmgreen requested review of this revision.Apr 26 2023, 10:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2023, 10:01 AM
Matt added a subscriber: Matt.Apr 26 2023, 3:30 PM

I'm tempted to suggest this is better done as a canonicalisation via DAGCombine? Otherwise we then have to start duplicating other instances of these patterns, for example within mla/mls patfrags.

With (add node:$op1, (vselect node:$pred, node:$op2, (SVEDup0))) having one less use of op1 than (vselect node:$pred, (add node:$op1, node:$op2), node:$op1), such a canonicalisation might have other benefits where combines/patterns are guarded by hasOneUse().

Hello. Yes, that was actually the plan, but the other way around. There is a function that can canonicalize in the DAG through shouldFoldSelectWithIdentityConstant, to the select(add) form. Instcombine will fold the other way to add(select(..)). See d321f3aa64b4eaedd790dafe974cfdc0517cb22b for example. It is probably simpler to go via that method than dealing with the identity values for all node types (especially fp types). The predicated instructions are not as general in SVE as they are in MVE (MVE doesn't require destructive operands for many operations), but I think it should be better in general so long as there can always use predicated moves. I have patches for that and some other operations like and/or/xor/mul/mla/mls, but they needed some extra tests.

I will put the last part up now, that enables shouldFoldSelectWithIdentityConstant for SVE. There is one case that looks a little worse, but has a pair of selects that could be simplified.

paulwalker-arm added inline comments.Jun 5 2023, 3:49 AM
llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
402–405

If you shuffle your patches can this use VSelectCommPredOrPassthruPatFrags?

409

As you're previous patch canonicalises to the new pattern, can this old pattern be removed?

dmgreen abandoned this revision.Jun 15 2023, 1:15 AM

Thanks. I had this one earlier in the sequence to add the patterns before D149967 enabled shouldFoldSelectWithIdentityConstant, to keep the two separate. D149967 removes the pattern again to use VSelectUnpredOrPassthruPatFrags, so I can fold this into there.