This is an archive of the discontinued LLVM Phabricator instance.

[ARM][MVE] Fixes for tail predication.
ClosedPublic

Authored by samparker on Dec 17 2019, 7:32 AM.

Details

Summary
  1. Fix an issue with the incorrect value being used for the number of elements being passed to [d|w]lstp. We were trying to check that the value was available at LoopStart, but this doesn't consider that the last instruction in the block could also define the register! Two helpers have been added to RDA for this.
  2. Insert some code to now try to move the element count def or the insertion point so that we can perform more tail predication.
  3. Related to (1), the same off-by-one could prevent us from generating a low-overhead loop when a mov lr could have been the last instruction in the block.
  4. Fix up some instruction attributes so that not all the low-overhead loop instructions are labelled as branches and terminators - as this is not true for dls/dlstp.

Diff Detail

Event Timeline

samparker created this revision.Dec 17 2019, 7:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2019, 7:32 AM

Can you upload the patch with context?

samparker updated this revision to Diff 234306.Dec 17 2019, 7:58 AM

whoopsie daisy

dmgreen added inline comments.Dec 18 2019, 12:55 AM
llvm/lib/Target/ARM/ARMInstrThumb2.td
5212–5213

I'm surprised we haven't need this earlier!

Can these 2 lines be be removed now?

llvm/test/CodeGen/Thumb2/LowOverheadLoops/unsafe-cpsr-loop-use.mir
145

What's this about?

samparker marked 2 inline comments as done.Dec 18 2019, 1:17 AM
samparker added inline comments.
llvm/lib/Target/ARM/ARMInstrThumb2.td
5212–5213

Cheers. Yeah, it's not until I've now enabled evaluating terminators that we're seeing it!

llvm/test/CodeGen/Thumb2/LowOverheadLoops/unsafe-cpsr-loop-use.mir
145

To make this test still test what it's designed for... This patch now allows this loop to be converted into a loloop, by using $lr = tMOVr in the preheader, so I've put a move in the loop, a use of lr, to prevent the transform from kicking in.

dmgreen accepted this revision.Dec 18 2019, 1:58 AM

OK. LGTM. Nice fixes.

One thing I did notice, we are sometimes relying on kill flags, but not updating them ourselves? Something to think about maybe.

This revision is now accepted and ready to land.Dec 18 2019, 1:58 AM
This revision was automatically updated to reflect the committed changes.