This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeVectorOps][AArch64][RISCV][X86] Use OpVT for ISD::SETCC in LegalizeVectorOps.
ClosedPublic

Authored by craig.topper on Apr 30 2023, 10:52 PM.

Details

Summary

Previously, LegalizeVectorOps used the result VT while LegalizeDAG
used the operand VT. This patch makes them both use the operand VT.

This also makes it consistent with how the default cost model works.

I've hacked the AArch64 cost model to maintain old behavior for some
f16 vectors.

Diff Detail

Event Timeline

craig.topper created this revision.Apr 30 2023, 10:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2023, 10:52 PM
craig.topper requested review of this revision.Apr 30 2023, 10:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2023, 10:52 PM

I think both the result and the operands should be legalized.
I'd suggest to set Action to the action for an operand only if action for the result
is Legal, analogous to first checking for condcode legality above.

I think both the result and the operands should be legalized.
I'd suggest to set Action to the action for an operand only if action for the result
is Legal, analogous to first checking for condcode legality above.

In most cases, the legalization is less about the exact types and just that all floating point or all integer comparisons need to be legalized.

For targets without legal i1 vector result types, the result type for a vector FP compare is the integer type with elements the same size as the FP elements type. For vector integer compares the source and dest would be the same type. So if we check both types, I think that would force FP compares to be custom handled even if only the integer compares need custom handling.

I would prefer to make FP compares be identified by FP types, and integer compares identified by integer types.

For targets without legal i1 vector result types, the result type for a vector FP compare is the integer type with elements the same size as the FP elements type. For vector integer compares the source and dest would be the same type. So if we check both types, I think that would force FP compares to be custom handled even if only the integer compares need custom handling.

Sounds reasonable.

I would prefer to make FP compares be identified by FP types, and integer compares identified by integer types.

Agreed - using the operand type whenever possible makes more sense to me.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
2603

Is there anyone intending to deal with this FIXME?

craig.topper added inline comments.May 5 2023, 11:56 AM
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
2603
dmgreen added inline comments.May 7 2023, 4:17 PM
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
2603

I can look into this. It sounds fine for the time being.

RKSimon accepted this revision.May 13 2023, 4:10 AM

LGTM - cheers

This revision is now accepted and ready to land.May 13 2023, 4:10 AM
This revision was landed with ongoing or failed builds.May 13 2023, 11:33 PM
This revision was automatically updated to reflect the committed changes.