Page MenuHomePhabricator

[Thumb] Make the load/store optimizer less conservative
ClosedPublic

Authored by mroth on Sep 22 2014, 9:19 AM.

Details

Reviewers
rengolin
jmolloy
Summary

This is a fixed version of rL208992 - all known bugs should be fixed and it passes all the tests I'm running.

For Thumb1, the load/store optimizer will currently only merge LDRs/STRs if the base register is dead after the merged LDM/STM, or if there is no writeback to the base register.

However, this is overly conservative. If the condition flags aren't live at the location where we want to merge, we can do a few other things:

  • Reset the base register writeback with a SUBS.
  • Try to update future instructions such as ADDS/SUBS with the base register (as long as their definition of the CPSR is also dead).

This patch adds a function that will try to go forward in the basic block and modify any instructions that use the new value of the base register with writeback applied. If it encounters any hazards, it resets the base register to the old value.

I'll try to find a test case for rL217881 as a follow-up commit.

Cheers
Moritz

Diff Detail

Event Timeline

mroth updated this revision to Diff 13937.Sep 22 2014, 9:19 AM
mroth retitled this revision from to [Thumb] Make the load/store optimizer less conservative.
mroth updated this object.
mroth edited the test plan for this revision. (Show Details)
mroth added reviewers: rengolin, jmolloy.
mroth set the repository for this revision to rL LLVM.
mroth added a subscriber: Unknown Object (MLST).
rengolin accepted this revision.Sep 24 2014, 5:18 AM
rengolin edited edge metadata.

Hi Moritz,

Thanks for running this over again. Apart from the nit pick (that was already there, so not a big deal), looks good.

cheers,
--renato

lib/Target/ARM/ARMLoadStoreOptimizer.cpp
178

nit pick, but getAM{3,5}Op() could be merged into a variable like the one above (or maybe conditionalise on isAM3 for both above) and just have OP == sub down below.

This revision is now accepted and ready to land.Sep 24 2014, 5:18 AM
mroth added a comment.Sep 24 2014, 9:46 AM

Hi Renato,

Thanks for the review. I've refactored the code as you suggested and committed as rL218386.

Cheers
Moritz

mroth closed this revision.Sep 24 2014, 10:06 AM