This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Combine fptoi.sat(fmul) to fixed point cvtf
ClosedPublic

Authored by dmgreen on Nov 4 2021, 9:49 AM.

Details

Summary

We already have patterns for fptosi and fptoui plus fmul to fixed point convert, this adds equivalent patterns for fptosi.sat and fptoui.sat, which should apply equally well for the legal saturating variants.

Diff Detail

Event Timeline

dmgreen created this revision.Nov 4 2021, 9:49 AM
dmgreen requested review of this revision.Nov 4 2021, 9:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 4 2021, 9:49 AM

If I am not mistaken, I don’t see any f16 test?

dmgreen updated this revision to Diff 385027.Nov 5 2021, 4:50 AM

Rebase over some newly added fp16 tests.

SjoerdMeijer added inline comments.Nov 5 2021, 5:09 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.td
3772

Do we need to predicate this rule and the next on HasFullFP16?

dmgreen added inline comments.Nov 5 2021, 5:44 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.td
3772

Hmm. We haven't for any of the others here above or below. I don't think the nodes will be legal if we don't have fullfp16 - so they will never be possible to match against. I can add them if you think that's cleaner.

SjoerdMeijer added inline comments.Nov 5 2021, 6:15 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.td
3772

So yeah, there's definitely some inconsistency here. Other rewrite rules, but indeed not around here, do have it predicated on fullfp16. I am not sure to be honest, but it looks cleaner to me (although I of course agree that the nodes won't be matched if fullfp16 isn't enabled).

dmgreen updated this revision to Diff 385066.Nov 5 2021, 7:30 AM

Add HasFullFP16 to these and nearby patterns.

SjoerdMeijer accepted this revision.Nov 5 2021, 7:45 AM

Cheers, LGTM.

This revision is now accepted and ready to land.Nov 5 2021, 7:45 AM
This revision was landed with ongoing or failed builds.Nov 8 2021, 2:07 AM
This revision was automatically updated to reflect the committed changes.