This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Avoid multiple PTRUE values for SETCC.
Needs ReviewPublic

Authored by sdesmalen on Feb 9 2022, 6:55 AM.

Details

Summary

Conversion between fixed- and scalable vector types may sometimes
lead to multiple PTRUE values to be used, one 'all active' and one
for the specific fixed-width vector length.

This patch canonicalises the predicate of SETCC_MERGE_ZERO to be
the more specific of the two, when it's inputs come from a conversion
of a fixed -> scalable type.

Diff Detail

Event Timeline

sdesmalen created this revision.Feb 9 2022, 6:55 AM
sdesmalen requested review of this revision.Feb 9 2022, 6:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2022, 6:55 AM
paulwalker-arm added inline comments.Feb 10 2022, 5:37 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
14106–14121

I thought we have an isel pattern to capture this. Is it the case you need to catch it early? If so then I think you should also remove the isel pattern to prove it's redundant.

17096

This also needs to test V.getValueType().isScalableVector() or have an equivalent assert if you never expect it to be called when such is not true.

17124

This looks misplaced?

17128–17129

This has me wondering if we're in fact missing something else. Do you know where the all active PTRUE is coming from?

You're making the assumption that there exists a fixed length PTRUE, which is probably true but still feels like an arbitrary assumption. For example, when vscale-min==vscale-max we favour all active PTRUEs.

peterwaller-arm added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
2386

Note: Block of similar forward decls here.

17092–17093

Nit. Is this the right place for this forward declaration? It's not used in the function immediately below. I see a block of forward decls above.

llvm/test/CodeGen/AArch64/sve-setcc.ll
119

Looks like a draft function name slipped in here.

Matt added a subscriber: Matt.Mar 17 2022, 5:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 5:56 PM
Allen added a subscriber: Allen.Mar 19 2022, 2:48 AM