Very conservative memory check in ShrinkWrap pass causing bad placement of prolog/Epilog

Code Generation
1

Hi All,

I am trying to use shrink-wrap for some early exit programs and understanding ShrinkWrap Pass. I found a check in shrinkWrapPass which is very conservative and bailed out, and missing the opportunity of early exit for which this optimization is indented.

The change is a four year old fix which was introduce to fix the violation as per AAPCS for AArch64 target. But It is made generic for all the other architecture also.

  if (MI.mayLoadOrStore())
    return true;

Does such kind of check applicable for X86 architecture also? Is it sound to enable this check for AArch64 only and relax the condition for other architecture?

Please let me know your views.

@dsampaio @davemgreen @qcolombet @fhahn @efriedma-quic @tera

:gear: D63152 [FIX] Forces shrink wrapping to consider any memory access as aliasing with the stack (llvm.org)

37472 – Shrink wrapping on AArch64 violates AAPCS (access below SP) (llvm.org)

2

Hi,
That’s a quite old one. I don’t know about the x86 calling convention, but I would imagine a premature stack restoring would affect most archs in possibly destroying an unprotected stack during an exception treatment. Perhaps @RKSimon could share his view from the x86 side. If you do have an arch that does not suffer from it, yes, adding a hook in the TargetLoweringInfo sounds as a good solution.
Cheers

1 Like
3

Hi,

This check is required for all architectures.
The way we could relax that is by checking that the related load/store doesn’t alias with the stack.

For instance, @fhahn made the fix less conservative with ⚙ D149668 [ShrinkWrap] Use underlying object to rule out stack access..

Cheers,
-Quentin

1 Like
4

Yep, the restriction in current main has been relaxed. @rohitaggarwal007 is it possible you are looking at an older version of LLVM?

See https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/ShrinkWrap.cpp#L319 for the current version of the check.

1 Like
5

I rebase the code and able to position the PUSHs in the desired place. The check is now less conservative.

Thank you all for the help.