This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Mark fixed-type FP extending/truncating loads/stores as custom
ClosedPublic

Authored by bsmith on Nov 25 2021, 4:12 AM.

Details

Summary

This allows the generic DAG combine to fold fp_extend/fp_trunc into
loads/stores which we can then lower into a integer extending
load/truncating store plus an FP_EXTEND/FP_ROUND.

The nuance here is that fixed-type FP_EXTEND/FP_ROUND require unpacked
types hence lowering them introduces an unpack/zip. By allowing these
nodes to be combined with loads/store we make it much easier to have
this unpack/zip combined into the load/store by our custom lowering.

Diff Detail

Event Timeline

bsmith created this revision.Nov 25 2021, 4:12 AM
bsmith requested review of this revision.Nov 25 2021, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 25 2021, 4:12 AM
paulwalker-arm added inline comments.Nov 25 2021, 5:42 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1563

This looks weird to me, shouldn't InnerVT be floating point? I guess the reason this works is because the i8 case is essentially bogus and you end up with the necessary floating point types from the second iteration. Either way I think this wants to be MVT::f16.

18680

Purely to help with formatting I believe you can just do VT.isFloatingPoint() here.

18681

Can you be specific here as in use == ISD::EXTLOAD as we should need to support any other extension types.

18688

I think it'll be safer to mirror the other if condition as the two are linked (i.e. we need to do both or none based on the same requirement).

bsmith added inline comments.Nov 25 2021, 6:00 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1563

I meant to change this but clearly forgot!

bsmith updated this revision to Diff 389763.Nov 25 2021, 6:22 AM
  • Fix incorrect type used in addTypeForFixedLengthSVE
  • Cleanup conditions in LowerFixedLengthVectorLoadToSVE
Matt added a subscriber: Matt.Nov 25 2021, 8:53 AM
paulwalker-arm accepted this revision.Nov 26 2021, 3:48 AM
paulwalker-arm added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
18673–18674

You could move this into the if block where it's used.

18770–18771

Perhaps TruncVT? Plus as above this could be moved into the if block where it is used.

18775

As above this can be just VT.

This revision is now accepted and ready to land.Nov 26 2021, 3:48 AM