This is an archive of the discontinued LLVM Phabricator instance.

ShrinkWrap: Ignore load/store until stack address computation
ClosedPublic

Authored by MatzeB on Jun 5 2023, 5:14 PM.

Details

Summary

No longer conservatively assume a load/store accesses the stack when we can prove that we did not compute any stack-relative address up to this point in the program.

We do this in a cheap not-quite-a-dataflow-analysis: Assume NoStackAddressUsed when all predecessors of a block already guarantee it. Process blocks in reverse post order to guarantee that except for loop headers we have processed all predecessors of a block before processing the block itself. For loops we accept the conservative answer as they are unlikely to be shrink-wrappable anyway.

Diff Detail

Event Timeline

MatzeB created this revision.Jun 5 2023, 5:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2023, 5:14 PM
MatzeB requested review of this revision.Jun 5 2023, 5:14 PM
MatzeB added inline comments.Jun 5 2023, 5:16 PM
llvm/lib/CodeGen/ShrinkWrap.cpp
304

Was wondering if its worth removing this complex expression with the patch in place, but it's hard to argue that there could be examples where this helps placing the epilogue, so I left it in for now...

MatzeB updated this revision to Diff 528647.Jun 5 2023, 5:33 PM
qcolombet accepted this revision.Jun 12 2023, 5:42 AM

Hi @MatzeB

The change looks good to me, but can we flip the logic NoStackAddressUsed => StackAddressUsed?

I find the !Noxxx or Noxxx = false difficult to reason about :).

Cheers,
-Quentin

This revision is now accepted and ready to land.Jun 12 2023, 5:42 AM
thegameg accepted this revision.Jun 12 2023, 6:54 PM

LG, thanks! Agreed with @qcolombet on the double negation.

MatzeB updated this revision to Diff 534664.Jun 26 2023, 11:20 AM

address review comments

This revision was landed with ongoing or failed builds.Jun 26 2023, 1:51 PM
This revision was automatically updated to reflect the committed changes.