[PATCH] D102342: [SPARC] Fix register reuse in leaf function remapping step

[Koakuma via Phabricator via llvm-commits]

koakuma added a comment.

In D102342#3060935 <https://reviews.llvm.org/D102342#3060935>, @dcederman wrote:

> Hello!
>
> It seems that this patch contains a rewrite of the remapRegsForLeafProc() function in addition to the new check described in the summary. Could you split it up into two patches, one with the rewrite and one with the added check?

I'm not sure about this. The way I see it is that the checks need to be done together with the rewrite, so it probably cannot be split like that (?)

> Is the problem that the isLeafProc() does not detect the case when inline assembly clobbers one of the output registers

No, the problem is that on functions with more than six parameters, when leaf function optimization is turned on, the function might be compiled in such a way that the loading of 7th-and-further parameters corrupts the value of other parameters.
This does not happen on functions with six or less parameters, even with inline assembly inside.

For example, on current LLVM, the testcase in the patch compiles to this:

  leaf_proc_give_up:
      ld [%sp+2227], %o5 # The original value of %o5 is lost here
                         # causing the inline assembly to run with
                         # the wrong register values.
      mov     %o0, %g1
      mov     %o1, %o0
      mov     %o2, %o1
      mov     %o3, %o2
      mov     %o4, %o3
      mov     %o5, %o4
      !APP
      !NO_APP
      retl
      nop

As far as I can tell, there are three solutions for this: (1) reorder the code so the loads happen after the movs, (2) load the parameter to a temporary register first then move it to %o5 after all the other movs are done, or (3) simply bail out of the optimization entirely, as is done in this patch.

> or have you been able to trigger it in some other way?

So far the only way I know to trigger this is with inline assembly (we encountered this when we're trying to write a system call wrapper for Linux).

> Does it make sense to put the extra check in isLeafProc() instead?

I guess it is possible to do that by flagging all functions that has more than six parametes as nonleaf (so the optimization wouldn't be triggered), but I'm afraid that it'd be too aggressive.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102342/new/

https://reviews.llvm.org/D102342