This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch i386]: Expand sibling-tail-calls via accumulator register


Hello Kai,

On 28 May 2014, at 09:43, Kai Tietz wrote:

> Index: gcc/config/i386/i386.c
> ===================================================================
> --- gcc/config/i386/i386.c	(revision 210985)
> +++ gcc/config/i386/i386.c	(working copy)
> @@ -38893,7 +38893,16 @@ x86_output_mi_thunk (FILE *file,
>      For our purposes here, we can get away with (ab)using a jump pattern,
>      because we're going to do no optimization.  */
>   if (MEM_P (fnaddr))
> -    emit_jump_insn (gen_indirect_jump (fnaddr));
> +    {
> +      if (sibcall_insn_operand (fnaddr, word_mode))
> +	{
> +	  tmp = gen_rtx_CALL (VOIDmode, fnaddr, const0_rtx);
> +          tmp = emit_call_insn (tmp);
> +          SIBLING_CALL_P (tmp) = 1;
> +	}
> +      else
> +	emit_jump_insn (gen_indirect_jump (fnaddr));
> +    }
>   else

As discussed in PR61387 and on IRC, this patch (http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=211089), in particular the section above, causes massive regressions (~900 test fails) for x86_64-darwin*.

I have some questions and observations and would request some progress in resolving this.

FWIW, x86_64-darwin *passes* the test-cases you added with the patch series.

====

The section of code above will only fire  (1) we are PIC (2) we are not PECOFF (3) the target returns binds_local_p () false for the function (see lines 38863-38871).

Observations:

A. AFAICT, the section of code above is _never_ exercised by x86_64-linux for a full bootstrap and regression test.
  -- so please could you identify how you tested it?
  -- whatever is done to resolve the current issue, it seems that an appropriate test-case is required.

B. You have considerably altered the behaviour of that code block without any amendment to the preceding comment.
  -- please update the comment to refect the changed behaviour.

the case is generated by:

	  tmp = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, fnaddr), UNSPEC_GOTPCREL);
	  tmp = gen_rtx_CONST (Pmode, tmp);
	  fnaddr = gen_const_mem (Pmode, tmp);

C. The code above seems pretty generic, grep doesn't reveal any different handling of UNSPEC_GOTPCREL for Darwin - what part of the Darwin implementation are you suggesting needs amendment?

====

Certainly, it is easy enough to make a patch to disable this operation on Darwin (and thus fix the regressions) -- however, I'd like to be sure that there is simply not some lurking issue, that has simply only manifested on Darwin so far.

thanks,
Iain


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]