Page MenuHomePhabricator

[ARM] Allow rounding intrinsics to be tail predicated
ClosedPublic

Authored by samtebbs on Jun 25 2020, 7:40 AM.

Details

Summary

This patch stops the trunc, rint, round, floor and ceil intrinsics from blocking tail predication.

Diff Detail

Event Timeline

samtebbs created this revision.Jun 25 2020, 7:40 AM
samtebbs updated this revision to Diff 273375.Jun 25 2020, 8:20 AM

Add missing floor and ceil tests

dmgreen added inline comments.Jun 25 2020, 8:41 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
298

Um, you might be fixing my bug here, but can you make it so that the floating point instructions only tail predicate when Subtarget->hasMVEEFloatOps() is true? Otherwise in integer only MVE we could start trying to and tail predicate where it will end up expanding the instruction (which probably isn't a huge deal, but we should try and get it correct).

llvm/test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-intrinsic-round.ll
4

Can you also add a test for nearbyint too, to show that it _doesn't_ get tail predicated (I think it gets expanded to a multiple scalar instructions.

260

I think you can remove all this and the code below. It complains about stuff then use -mattr=+mve.fp in the run line.

You will probably have to remove the !tbaa info too.

samtebbs updated this revision to Diff 273645.Jun 26 2020, 3:25 AM
samtebbs marked an inline comment as done.

Only tail predicate if the subtarget has float ops, add nearbyint test and clean up the test file.

samtebbs marked 3 inline comments as done.Jun 26 2020, 3:26 AM
samtebbs added inline comments.
llvm/test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-intrinsic-round.ll
260

I did try removing them before but ran into an abort, which https://reviews.llvm.org/D82292 has since fixed.

SjoerdMeijer added inline comments.Jun 26 2020, 5:09 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
111

It would have my preference not to pass ST around all the time; just set it as a class member or get it in IsPredicatedVectorLoop.

dmgreen added inline comments.Jun 26 2020, 5:28 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
271–272

fma can go into the float block too.

llvm/test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-intrinsic-round.ll
230

You can use the same test code as floor, for example, but with floor replaced by nearbyint. This won't test that the vecotrizer does or doesn't tail predicate it, but will test that we expand to something non-tail predicated still.

samtebbs updated this revision to Diff 274412.Jun 30 2020, 4:10 AM
samtebbs marked an inline comment as done.

Fix up nearbyint test, make ST a class member and move fma case.

samtebbs marked 3 inline comments as done.Jun 30 2020, 4:11 AM
samtebbs updated this revision to Diff 274475.Jun 30 2020, 7:43 AM

Fix types of nearbyint intrinsic

dmgreen accepted this revision.Jun 30 2020, 8:24 AM

Thanks. LGTM with one nit.

llvm/lib/Target/ARM/MVETailPredication.cpp
284

I think you may need to add a LLVM_FALLTHROUGH (or a extra break) to stop a warning from cropping up.

This revision is now accepted and ready to land.Jun 30 2020, 8:24 AM
samtebbs updated this revision to Diff 274499.Jun 30 2020, 8:37 AM
samtebbs marked an inline comment as done.

Add LLVM_FALLTHROUGH

llvm/lib/Target/ARM/MVETailPredication.cpp
284

Thanks. Well spotted.

samtebbs marked an inline comment as done.Jun 30 2020, 8:37 AM
This revision was automatically updated to reflect the committed changes.