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

Hi,

I modified prior patch so that it uses the new predicate sibcall_memory_operand to extend sibcall_insn_operand.
Just one change in i386.c remains about x86_output_mi_thunk.  Later one isn't pretty much essential.  Nevertheless it makes
code equivalent to none-memory-case for potential tail-sibcalls.

ChangeLog gcc

2014-05-28  Kai Tietz  <ktietz@redhat.com>

	* config/i386/i386.c (x86_output_mi_thunk): Add memory case
	for sibling-tail-calls.
	* config/i386/i386.md (sibcall_insn_operand): Add memory-constrain
	to its use.
	* config/i386/predicates.md (sibcall_memory_operand): New predicate.
	(sibcall_insn_operand): Add check for sibcall_memory_operand.

ChangeLog testsuite

2014-05-28  Kai Tietz  <ktietz@redhat.com>

	PR target/60104
	* gcc.target/i386/sibcall-1.c: New test.
	* gcc.target/i386/sibcall-2.c: New test.
	* gcc.target/i386/sibcall-3.c: New test.

Regression tested x86_64-unknown-linux-gnu multilib, x86_64-w64-mingw32, and i686-pc-cygwin.  Ok for apply?

Regards,
Kai

On Wed, May 28, 2014 at 1:43 AM, Kai Tietz <ktietz@redhat.com> wrote:
> Hi,
>
> I modified prior patch so that it uses the new predicate sibcall_memory_operand to extend sibcall_insn_operand.
> Just one change in i386.c remains about x86_output_mi_thunk.  Later one isn't pretty much essential.  Nevertheless it makes
> code equivalent to none-memory-case for potential tail-sibcalls.
>
> ChangeLog gcc
>
> 2014-05-28  Kai Tietz  <ktietz@redhat.com>
>
>         * config/i386/i386.c (x86_output_mi_thunk): Add memory case
>         for sibling-tail-calls.
>         * config/i386/i386.md (sibcall_insn_operand): Add memory-constrain
>         to its use.
>         * config/i386/predicates.md (sibcall_memory_operand): New predicate.
>         (sibcall_insn_operand): Add check for sibcall_memory_operand.
>
> ChangeLog testsuite
>
> 2014-05-28  Kai Tietz  <ktietz@redhat.com>
>
>         PR target/60104
>         * gcc.target/i386/sibcall-1.c: New test.
>         * gcc.target/i386/sibcall-2.c: New test.
>         * gcc.target/i386/sibcall-3.c: New test.
>
> Regression tested x86_64-unknown-linux-gnu multilib, x86_64-w64-mingw32, and i686-pc-cygwin.  Ok for apply?
>
> Regards,
> Kai
>
>
> 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
>      {
>        if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
> Index: gcc/config/i386/i386.md
> ===================================================================
> --- gcc/config/i386/i386.md     (revision 210985)
> +++ gcc/config/i386/i386.md     (working copy)
> @@ -11376,7 +11376,7 @@
>    [(set_attr "type" "call")])
>
>  (define_insn "*sibcall"
> -  [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "Uz"))
> +  [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "Uzm"))
>          (match_operand 1))]
>    "SIBLING_CALL_P (insn)"
>    "* return ix86_output_call_insn (insn, operands[0]);"
> @@ -11406,7 +11406,7 @@
>    [(set_attr "type" "call")])
>
>  (define_insn "*sibcall_pop"
> -  [(call (mem:QI (match_operand:SI 0 "sibcall_insn_operand" "Uz"))
> +  [(call (mem:QI (match_operand:SI 0 "sibcall_insn_operand" "Uzm"))
>          (match_operand 1))
>     (set (reg:SI SP_REG)
>         (plus:SI (reg:SI SP_REG)
> @@ -11451,7 +11451,7 @@
>
>  (define_insn "*sibcall_value"
>    [(set (match_operand 0)
> -       (call (mem:QI (match_operand:W 1 "sibcall_insn_operand" "Uz"))
> +       (call (mem:QI (match_operand:W 1 "sibcall_insn_operand" "Uzm"))
>               (match_operand 2)))]
>    "SIBLING_CALL_P (insn)"
>    "* return ix86_output_call_insn (insn, operands[1]);"
> @@ -11494,7 +11494,7 @@
>
>  (define_insn "*sibcall_value_pop"
>    [(set (match_operand 0)
> -       (call (mem:QI (match_operand:SI 1 "sibcall_insn_operand" "Uz"))
> +       (call (mem:QI (match_operand:SI 1 "sibcall_insn_operand" "Uzm"))
>               (match_operand 2)))
>     (set (reg:SI SP_REG)
>         (plus:SI (reg:SI SP_REG)
> Index: gcc/config/i386/predicates.md
> ===================================================================
> --- gcc/config/i386/predicates.md       (revision 210985)
> +++ gcc/config/i386/predicates.md       (working copy)
> @@ -71,6 +71,16 @@
>    return ANY_QI_REG_P (op);
>  })
>
> +(define_predicate "sibcall_memory_operand"
> +  (match_operand 0 "memory_operand")
> +{
> +  op = XEXP (op, 0);
> +
> +  if (GET_CODE (op) == CONST)
> +    op = XEXP (op, 0);
> +  return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));
> +})
> +
>  ;; Match an SI or HImode register for a zero_extract.
>  (define_special_predicate "ext_register_operand"
>    (match_operand 0 "register_operand")
> @@ -600,7 +610,9 @@
>  (define_special_predicate "sibcall_insn_operand"
>    (ior (match_test "constant_call_address_operand
>                      (op, mode == VOIDmode ? mode : Pmode)")
> -       (match_operand 0 "register_no_elim_operand")))
> +       (match_operand 0 "register_no_elim_operand")
> +       (and (not (match_test "TARGET_X32"))
> +           (match_operand 0 "sibcall_memory_operand"))))
>
>  ;; Return true if OP is a call from MS ABI to SYSV ABI function.
>  (define_predicate "call_rex64_ms_sysv_operation"
> Index: gcc/testsuite/gcc.target/i386/sibcall-1.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/sibcall-1.c   (revision 0)
> +++ gcc/testsuite/gcc.target/i386/sibcall-1.c   (working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-O2" } */
> +
> +extern int (*foo)(int);
> +
> +int boo (int a)
> +{
> +  return (*foo) (a);
> +}
> +
> +/* { dg-final { scan-assembler-not "mov" } } */
> Index: gcc/testsuite/gcc.target/i386/sibcall-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/sibcall-2.c   (revision 0)
> +++ gcc/testsuite/gcc.target/i386/sibcall-2.c   (working copy)
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { xfail { *-*-* } } } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-O2" } */
> +
> +extern int doo1 (int);
> +extern int doo2 (int);
> +extern void bar (char *);
> +
> +int foo (int a)
> +{
> +  char s[256];
> +  bar (s);
> +  return (a < 0 ? doo1 : doo2) (a);
> +}
> +
> +/* { dg-final { scan-assembler-not "call[ \t]*.%eax" } } */
> Index: gcc/testsuite/gcc.target/i386/sibcall-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/sibcall-3.c   (revision 0)
> +++ gcc/testsuite/gcc.target/i386/sibcall-3.c   (working copy)
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-O2" } */
> +
> +extern
> +#ifdef _WIN32
> + __declspec (dllimport)
> +#endif
> + void foo (int a);
> +
> +void bar (int a)
> +{
> +  return foo (a);
> +}
> +
> +/* { dg-final { scan-assembler-not "jmp[ \t]*.%eax" } } */
> Index: gcc/testsuite/gcc.target/i386/sibcall-4.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/sibcall-4.c   (revision 0)
> +++ gcc/testsuite/gcc.target/i386/sibcall-4.c   (working copy)
> @@ -0,0 +1,15 @@
> +/* Testcase for PR target/46219.  */
> +/* { dg-do compile { xfail { *-*-* } } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-O2" } */
> +
> +typedef void (*dispatch_t)(long offset);
> +
> +dispatch_t dispatch[256];
> +
> +void male_indirect_jump (long offset)
> +{
> +  dispatch[offset](offset);
> +}
> +
> +/* { dg-final { scan-assembler-not "jmp[ \t]*.%eax" } } */

Shouldn't the testcases also be applied to x86-64?

On 05/28/2014 01:43 AM, Kai Tietz wrote:
> +  if (GET_CODE (op) == CONST)
> +    op = XEXP (op, 0);
> +  return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));

Surely all this boils down to just CONSTANT_P (op),
without having to look through the CONST at all.

Otherwise this looks ok.


r~

On 05/28/14 11:22, Richard Henderson wrote:
> On 05/28/2014 01:43 AM, Kai Tietz wrote:
>> +  if (GET_CODE (op) == CONST)
>> +    op = XEXP (op, 0);
>> +  return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));
>
> Surely all this boils down to just CONSTANT_P (op),
> without having to look through the CONST at all.
Something certainly seems odd there.

Kai, is your intention to detect symbol_ref + const_int?  Isn't the 
canonical form for that:

(const (plus (symbol_ref) (const_int))?

It appears to me the code above looks for

(const (symbol_ref))

or

(const (const_int))

?!?

Jeff

----- Original Message -----
> On 05/28/14 11:22, Richard Henderson wrote:
> > On 05/28/2014 01:43 AM, Kai Tietz wrote:
> >> +  if (GET_CODE (op) == CONST)
> >> +    op = XEXP (op, 0);
> >> +  return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));
> >
> > Surely all this boils down to just CONSTANT_P (op),
> > without having to look through the CONST at all.
> Something certainly seems odd there.
> 
> Kai, is your intention to detect symbol_ref + const_int?  Isn't the
> canonical form for that:
> 
> (const (plus (symbol_ref) (const_int))?
> 
> It appears to me the code above looks for
> 
> (const (symbol_ref))
> 
> or
> 
> (const (const_int))
> 
> ?!?
> 
> Jeff
> 

Yes, I missed the plus-part.

I am just running bootstrap with regression testing for altering predicate to:

(define_predicate "sibcall_memory_operand"
  (match_operand 0 "memory_operand")
{
  op = XEXP (op, 0);

  if (GET_CODE (op) == CONST)
    op = XEXP (op, 0);
  if (GET_CODE (op) == PLUS && CONSTANT_P (XEXP (op, 0)))
    op = XEXP (op, 1);
  return CONSTANT_P (op);
})

Kai

On Wed, May 28, 2014 at 05:28:31PM -0400, Kai Tietz wrote:
> Yes, I missed the plus-part.
> 
> I am just running bootstrap with regression testing for altering predicate to:
> 
> (define_predicate "sibcall_memory_operand"
>   (match_operand 0 "memory_operand")
> {
>   op = XEXP (op, 0);
> 
>   if (GET_CODE (op) == CONST)
>     op = XEXP (op, 0);
>   if (GET_CODE (op) == PLUS && CONSTANT_P (XEXP (op, 0)))
>     op = XEXP (op, 1);

Why not get rid of all the above 4 lines and just keep:

>   return CONSTANT_P (op);

?  CONST matches CONSTANT_P, and what is inside of CONST should be
fine, and (plus (symbol_ref) (const_int)) not surrounded by CONST
ir invalid.

	Jakub

On 05/28/14 15:52, Jakub Jelinek wrote:
> On Wed, May 28, 2014 at 05:28:31PM -0400, Kai Tietz wrote:
>> Yes, I missed the plus-part.
>>
>> I am just running bootstrap with regression testing for altering predicate to:
>>
>> (define_predicate "sibcall_memory_operand"
>>    (match_operand 0 "memory_operand")
>> {
>>    op = XEXP (op, 0);
>>
>>    if (GET_CODE (op) == CONST)
>>      op = XEXP (op, 0);
>>    if (GET_CODE (op) == PLUS && CONSTANT_P (XEXP (op, 0)))
>>      op = XEXP (op, 1);
>
> Why not get rid of all the above 4 lines and just keep:
>
>>    return CONSTANT_P (op);
>
> ?  CONST matches CONSTANT_P, and what is inside of CONST should be
> fine, and (plus (symbol_ref) (const_int)) not surrounded by CONST
> ir invalid.
Haven't we recently had problems with being overly accepting of stuff 
inside CONST when using the CONST for address expressions.  ISTM we 
should only accept what the processor supports here.

jeff

On Wed, May 28, 2014 at 03:54:58PM -0600, Jeff Law wrote:
> >Why not get rid of all the above 4 lines and just keep:
> >
> >>   return CONSTANT_P (op);
> >
> >?  CONST matches CONSTANT_P, and what is inside of CONST should be
> >fine, and (plus (symbol_ref) (const_int)) not surrounded by CONST
> >ir invalid.
> Haven't we recently had problems with being overly accepting of
> stuff inside CONST when using the CONST for address expressions.
> ISTM we should only accept what the processor supports here.

The only recent problem I remember was that we forgot to put CONST
around (plus (symbol_ref) (const_int)), but I see no problem not
accepting such invalid RTL.
The processor shouldn't care, for the instructions a CONST is just
any kind of immediate, what exactly it is matters solely to the
assembler/linker and dynamic linker
(if there are relocations for it, if the expression can be expressed in
the assembly, etc.).  But that is common to all CONST operands, there is
nothing special particularly about sibcalls.

	Jakub

----- Original Message -----
> On Wed, May 28, 2014 at 03:54:58PM -0600, Jeff Law wrote:
> > >Why not get rid of all the above 4 lines and just keep:
> > >
> > >>   return CONSTANT_P (op);
> > >
> > >?  CONST matches CONSTANT_P, and what is inside of CONST should be
> > >fine, and (plus (symbol_ref) (const_int)) not surrounded by CONST
> > >ir invalid.
> > Haven't we recently had problems with being overly accepting of
> > stuff inside CONST when using the CONST for address expressions.
> > ISTM we should only accept what the processor supports here.
> 
> The only recent problem I remember was that we forgot to put CONST
> around (plus (symbol_ref) (const_int)), but I see no problem not
> accepting such invalid RTL.
> The processor shouldn't care, for the instructions a CONST is just
> any kind of immediate, what exactly it is matters solely to the
> assembler/linker and dynamic linker
> (if there are relocations for it, if the expression can be expressed in
> the assembly, etc.).  But that is common to all CONST operands, there is
> nothing special particularly about sibcalls.
> 
> 	Jakub
> 

Well, actually we want to prevent to accept anything plus/mult within memory-addresses, which hasn't a symbol-ref, or a constant-value as arguments.  Is it for sure that there are within a CONST-rtx no registers?  If so, we could check intitally for CONSTANT_P.

Kai

On 05/28/2014 02:54 PM, Jeff Law wrote:
> On 05/28/14 15:52, Jakub Jelinek wrote:
>> On Wed, May 28, 2014 at 05:28:31PM -0400, Kai Tietz wrote:
>>> Yes, I missed the plus-part.
>>>
>>> I am just running bootstrap with regression testing for altering predicate to:
>>>
>>> (define_predicate "sibcall_memory_operand"
>>>    (match_operand 0 "memory_operand")
>>> {
>>>    op = XEXP (op, 0);
>>>
>>>    if (GET_CODE (op) == CONST)
>>>      op = XEXP (op, 0);
>>>    if (GET_CODE (op) == PLUS && CONSTANT_P (XEXP (op, 0)))
>>>      op = XEXP (op, 1);
>>
>> Why not get rid of all the above 4 lines and just keep:
>>
>>>    return CONSTANT_P (op);
>>
>> ?  CONST matches CONSTANT_P, and what is inside of CONST should be
>> fine, and (plus (symbol_ref) (const_int)) not surrounded by CONST
>> ir invalid.
> Haven't we recently had problems with being overly accepting of stuff inside
> CONST when using the CONST for address expressions.  ISTM we should only accept
> what the processor supports here.

Recall that it has just satisfied memory_operand, where all the real checks
should have been done.  I think just the CONSTANT_P check is sufficient.


r~

So, completed bootstrap and regression-test for x86_64-unknown-linux-gnu (multilib) by using following predicate for sibcall-patch.

(define_predicate "sibcall_memory_operand"
  (match_operand 0 "memory_operand")
{
  return CONSTANT_P (op);
})


Worked fine, no regressions.  Is sibcall-patch ok with that predicate to be applied?

Regards,
Kai

On 05/30/2014 01:08 AM, Kai Tietz wrote:
> (define_predicate "sibcall_memory_operand"
>   (match_operand 0 "memory_operand")
> {
>   return CONSTANT_P (op);
> })

Surely that always returns false?  Surely XEXP (op, 0) so that you look at the
address, not the memory.


r~

----- Original Message -----
> On 05/30/2014 01:08 AM, Kai Tietz wrote:
> > (define_predicate "sibcall_memory_operand"
> >   (match_operand 0 "memory_operand")
> > {
> >   return CONSTANT_P (op);
> > })
> 
> Surely that always returns false?  Surely XEXP (op, 0) so that you look at
> the
> address, not the memory.
> 
> 
> r~
> 

Doh ... of course


(define_predicate "sibcall_memory_operand"
  (match_operand 0 "memory_operand")
{
  return CONSTANT_P (XEXP (op, 0));
})


Actually I tested the proper version :}

Kai

On 05/30/14 10:51, Kai Tietz wrote:
> ----- Original Message -----
>> On 05/30/2014 01:08 AM, Kai Tietz wrote:
>>> (define_predicate "sibcall_memory_operand"
>>>    (match_operand 0 "memory_operand")
>>> {
>>>    return CONSTANT_P (op);
>>> })
>>
>> Surely that always returns false?  Surely XEXP (op, 0) so that you look at
>> the
>> address, not the memory.
>>
>>
>> r~
>>
>
> Doh ... of course
>
>
> (define_predicate "sibcall_memory_operand"
>    (match_operand 0 "memory_operand")
> {
>    return CONSTANT_P (XEXP (op, 0));
> })
>
>
> Actually I tested the proper version :}
In that case, it's good to go.  OK for the trunk.

jeff

On Fri, May 30, 2014 at 9:51 AM, Kai Tietz <ktietz@redhat.com> wrote:
> ----- Original Message -----
>> On 05/30/2014 01:08 AM, Kai Tietz wrote:
>> > (define_predicate "sibcall_memory_operand"
>> >   (match_operand 0 "memory_operand")
>> > {
>> >   return CONSTANT_P (op);
>> > })
>>
>> Surely that always returns false?  Surely XEXP (op, 0) so that you look at
>> the
>> address, not the memory.
>>
>>
>> r~
>>
>
> Doh ... of course
>
>
> (define_predicate "sibcall_memory_operand"
>   (match_operand 0 "memory_operand")
> {
>   return CONSTANT_P (XEXP (op, 0));
> })
>
>
> Actually I tested the proper version :}
>

Have you tested bootstrap on i686?  I think it may have broken bootstrap
on i686:

https://gcc.gnu.org/ml/gcc-regression/2014-05/msg00408.html

On Fri, May 30, 2014 at 3:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, May 30, 2014 at 9:51 AM, Kai Tietz <ktietz@redhat.com> wrote:
>> ----- Original Message -----
>>> On 05/30/2014 01:08 AM, Kai Tietz wrote:
>>> > (define_predicate "sibcall_memory_operand"
>>> >   (match_operand 0 "memory_operand")
>>> > {
>>> >   return CONSTANT_P (op);
>>> > })
>>>
>>> Surely that always returns false?  Surely XEXP (op, 0) so that you look at
>>> the
>>> address, not the memory.
>>>
>>>
>>> r~
>>>
>>
>> Doh ... of course
>>
>>
>> (define_predicate "sibcall_memory_operand"
>>   (match_operand 0 "memory_operand")
>> {
>>   return CONSTANT_P (XEXP (op, 0));
>> })
>>
>>
>> Actually I tested the proper version :}
>>
>
> Have you tested bootstrap on i686?  I think it may have broken bootstrap
> on i686:
>
> https://gcc.gnu.org/ml/gcc-regression/2014-05/msg00408.html
>

The function is

void
ira_traverse_loop_tree (bool bb_p, ira_loop_tree_node_t loop_node,
                        void (*preorder_func) (ira_loop_tree_node_t),
                        void (*postorder_func) (ira_loop_tree_node_t))
{
    ...
    if (postorder_func != NULL)
      (*postorder_func) (loop_node);
}

postorder_func is passed on stack for i686 and we generate

   jmp    *0x28(%esp)

sibcall invalidates the original arguments on stack and puts
the new argument on stack.  There is no place on stack to
be used for indirect sibcall via memory.  In this case,
sibcall_memory_operand should return false.

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