This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Replace arm_neon_vqadds with sadd_sat
ClosedPublic

Authored by dmgreen on Oct 23 2019, 10:20 AM.

Details

Summary

This replaces the A32 NEON vqadds, vqaddu, vqsubs and vqsubu intrinsics with the target independent sadd_sat, uadd_sat, ssub_sat and usub_sat. This helps generate vqadds from standard IR nodes, which might be produced from the vectoriser. The old variants are removed in the process.

Diff Detail

Event Timeline

dmgreen created this revision.Oct 23 2019, 10:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 23 2019, 10:20 AM
dmgreen updated this revision to Diff 226222.Oct 24 2019, 4:05 AM

Remove some dead code.

SjoerdMeijer added inline comments.Nov 11 2019, 5:54 AM
llvm/test/CodeGen/ARM/addsubo-legalization.ll
88

Nit, just curious why this one is unaffected. Looking at the changes below, I was perhaps expecting similar changes here.

dmgreen marked an inline comment as done.Nov 12 2019, 12:48 AM
dmgreen added inline comments.
llvm/test/CodeGen/ARM/addsubo-legalization.ll
88

Good question. It looks like the default expansion for SADD/SUBO can go via a SADD/SUBSAT, but the UADD/SUBO prefers to go via ADD/SUBCARRY.

We could add expansion for UADDO to UADDSAT, but I think it's best not to do that here.

SjoerdMeijer accepted this revision.Nov 12 2019, 1:17 AM
SjoerdMeijer added inline comments.
llvm/test/CodeGen/ARM/addsubo-legalization.ll
88

Cheers, agreed, LGTM

This revision is now accepted and ready to land.Nov 12 2019, 1:17 AM
dmgreen updated this revision to Diff 229717.Nov 17 2019, 4:02 AM

I was about to commit this but as its removing old intrinsics it deserves some update code. I've added that now along with a test.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 27 2019, 5:39 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript