[PATCH, rs6000]: mark clobber for registers changed by untpyed_call

Segher Boessenkool segher@kernel.crashing.org
Wed Feb 5 19:25:00 GMT 2020


On Wed, Feb 05, 2020 at 09:53:27PM +0800, Jiufu Guo wrote:
> As PR93047 said, __builtin_apply/__builtin_return does not work well with
> -frename-registers.  This is caused by return register(e.g. r3) is used to
> rename another register, before return register is stored to stack.
> 
> This patch fix this issue by emitting clobber for those egisters which
> maybe changed by untyped call.

Yeah.  untyped_call does

  /* The optimizer does not know that the call sets the function value
     registers we stored in the result block.  We avoid problems by
     claiming that all hard registers are used and clobbered at this
     point.  */
  emit_insn (gen_blockage ());

but blockage does not say registers are used and clobbered:

@cindex @code{blockage} instruction pattern
@item @samp{blockage}
This pattern defines a pseudo insn that prevents the instruction
scheduler and other passes from moving instructions and using register
equivalences across the boundary defined by the blockage insn.
This needs to be an UNSPEC_VOLATILE pattern or a volatile ASM.

Many archs have this same implementation of untyped_call (and of
blockage, too).  It all just works by luck (or it doesn't work).

What we have is:

  emit_call_insn (gen_call (operands[0], const0_rtx, const0_rtx));

  for (i = 0; i < XVECLEN (operands[2], 0); i++)
    {
      rtx set = XVECEXP (operands[2], 0, i);
      emit_move_insn (SET_DEST (set), SET_SRC (set));
    }

... and nothing in the rtl stream says that those return registers are
actually set by that call.  Maybe we should use gen_call_value?  Can we
ever be asked to return more than a single thing here?

Some trivial patch comments:

> gcc/
> 2020-02-05  Jiufu Guo  <guojiufu@linux.ibm.com>
> 
> 	PR target/93047
> 	* config/rs6000/rs6000.md (untyped_call): add emit_clobber.

"Add", capital.

> gcc/testsuite
> 2020-02-05  Jiufu Guo  <guojiufu@linux.ibm.com>
> 
> 	PR target/93047
> 	* gcc.dg/torture/stackalign/builtin-return-2.c: New case.

"New test case."  (And there is trailing whitespace here; Git warns
about that, so this won't happen much in the future :-) )


Segher



More information about the Gcc-patches mailing list