Page MenuHomePhabricator

[ARM] Expand distributing increments to also handle existing pre/post inc instructions.
ClosedPublic

Authored by dmgreen on Jul 8 2020, 2:57 AM.

Details

Summary

This extends the distributing postinc code in load/store optimizer to also handle the case where there is an existing pre/post inc instuction, where subsequent instructions can be modified to use the adjusted increment from the increment. This can save us having to keep the old register live past the increment instruction.

Diff Detail

Event Timeline

dmgreen created this revision.Jul 8 2020, 2:57 AM
samparker added inline comments.Aug 7 2020, 1:08 AM
llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
2607

The function name doesn't appear to match that opcodes that it's checking?

2819–2821

I'm still a bit concerned with this loop.... Can't we get in a pickle if there are two or more instructions within the loop which use BaseReg with a zero offset? OtherAccesses relies on BaseAccess only being assigned once, right? Don't we also need to ensure that BaseReg isn't assigned between BaseAccess and OtherAccesses?

dmgreen updated this revision to Diff 291878.Sep 15 2020, 5:51 AM

Rebase and rename backwards function.

dmgreen added inline comments.Sep 15 2020, 5:53 AM
llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
2607

Oh. Whoops!

2819–2821

If I understand what you are saying properly.. We are in SSA form still, in this pre-ra pass. We shouldn't be reassigning BaseReg.

But I may have missed the point and it's been a while since I wrote this. Let me know.

Thanks, LGTM

llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
2819–2821

Ah! Yes, I had completely missed the 'ARMPreAllocLoadStoreOpt::'

samparker accepted this revision.Sep 17 2020, 12:48 AM
This revision is now accepted and ready to land.Sep 17 2020, 12:48 AM