Page MenuHomePhabricator

[RDA] Switch isSafeToMove iterators
ClosedPublic

Authored by samparker on Sep 28 2020, 7:50 AM.

Details

Summary

So that forwards is forwards and backwards is reverse. Also add a check so that we know the instructions are in the expected order. We'll need a follow-up patch to two to be able to successfully move instructions around for ARMLowOverheadLoops.

Diff Detail

Event Timeline

samparker created this revision.Sep 28 2020, 7:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 28 2020, 7:50 AM
samparker requested review of this revision.Sep 28 2020, 7:50 AM
samtebbs added inline comments.Sep 28 2020, 8:54 AM
llvm/test/CodeGen/Thumb2/LowOverheadLoops/mov-operand.ll
31

I may be misunderstanding this but won't this overwrite the subtraction to r3 that is done each iteration? So I think this should be outside the loop like it was before.

samparker added inline comments.Sep 29 2020, 12:21 AM
llvm/test/CodeGen/Thumb2/LowOverheadLoops/mov-operand.ll
31

The DLS doesn't actually signal the beginning on the loop, and the mov is still in the preheader. The loop body begins with the vctp.32.

SjoerdMeijer accepted this revision.Sep 29 2020, 1:07 AM

If Sam has no further questions, this looks good to me.

llvm/lib/CodeGen/ReachingDefAnalysis.cpp
582–587

This code is an exact copy of the previous one, except one iterator. This can probably be refactored into one IsSafeToMove function that gets passed in this iterator. But it's a nit, up to you if it is worth doing this.

This revision is now accepted and ready to land.Sep 29 2020, 1:07 AM
samparker added inline comments.Sep 29 2020, 1:18 AM
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
582–587

I thought the same, but each loop uses different sentinels: begin/rbegin and end/rend and I didn't see a way to handle that.

SjoerdMeijer added inline comments.Sep 29 2020, 1:19 AM
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
582–587

ok, fair enough

This revision was automatically updated to reflect the committed changes.