[llvm-dev] Issue with shrink wrapping

[Momchil Velikov via llvm-dev]

Hello,

So, I have this testcase:

    void f(int n, int x[]) {
      if (n < 0)
        return;

      int a[n];

      for (int i = 0; i < n; i++)
        a[i] = x[n - i - 1];

      for (int i = 0; i < n; i++)
        x[i] = a[i] + 1;
    }

that, compiled with -O1/-Os for AArch64 and X86, generates machine code,
which
fails to properly restore the stack pointer upon function return.

The testcase allocates a VLA, thus clang generates calls to
`llvm.stacksave` and
`llvm.stackrestore`. The latter call is lowered to `mov sp, x2`, however
this
move does not affect decisions made by the shrink wrapping pass, as the
instruction does not use or define a callee-saved register.

The end effect is that placement of register save/restore code is such that
along a certain path, the callee-saved registers and the stack pointer are
restored, but then the stack pointer is overwritten with an incorrect value.

I've "fixed" this by modifying `ShrinkWrap::useOrDefCSROrFI` to explicitly
check
for the stack pointer register (using
`TLI.getStackPointerRegisterToSaveRestore`) and also to ignore tall call
instructions (`isCall() && isReturn()`), since they implictly use SP (for
AArch{32,64} at least).

Does this look correct? Are there alternatives?

Shouldn't `ShrinkWrap::useOrDefCSROrFI` also check whether or not
`MachineInstr::Frame{Setup,Destroy}` flags are set?

In that case, I suppose an alternative slution would be to ensure that
`llvm.{stacksave,stackrestore} are ultimately lowered to `MachineInstr`s,
which
have these flags set, perhaps by custom lowering of
ISD::{STACKSAVE,STACKRESTORE} to pseudo-instructions? That'd involve
changes to
each backend, where shrink wrapping is enabled, although it looks least
hackish.

Comments?

Thanks and best regards,
Momchil Velikov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180409/3aafd649/attachment.html>