This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Cleanup for the MVETailPrediction pass
ClosedPublic

Authored by dmgreen on Nov 20 2020, 5:46 AM.

Details

Summary

I have had this patch from a while back when looking at the MVETailPredication pass. It strips out a lot of the code that should no longer be needed, leaving the important part of the pass - find active lane mask instructions and convert them to VCTP operations.

Diff Detail

Event Timeline

dmgreen created this revision.Nov 20 2020, 5:46 AM
dmgreen requested review of this revision.Nov 20 2020, 5:46 AM
samtebbs accepted this revision.Nov 20 2020, 6:06 AM

Nice, LGTM

This revision is now accepted and ready to land.Nov 20 2020, 6:06 AM
SjoerdMeijer added inline comments.Nov 20 2020, 6:09 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
351–352

nit: looks more than 80 columns...

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

Intially I was expecting this to be a NFC, but looks like it is also doing something good for codegen and we get more tail-predication. Why is that? Is that because of the dead code removal that is no longer in the way?

dmgreen added inline comments.Nov 20 2020, 6:50 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
351–352

Ah yeah. Will do.

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

IsPredicatedVectorLoop would be checking for certain instructions, and only convert the active lane mask if it finds them. This included intrinsics which it did not recognize as vector instructions.

It will always be better to convert to a VCTP than the to expand the active lane mask though, even if the loop does not get tail predicated successfully.

Also, as can be seen in this case, it sometimes gets intrinsics wrong. This is serialized, but that does not block tail predication.

SjoerdMeijer added inline comments.Nov 20 2020, 7:24 AM
llvm/test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-intrinsic-round.ll
244

Ah yes, got it, cheers.

Like Sam said, LGTM

This revision was automatically updated to reflect the committed changes.