Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Segher Boessenkool]

Hi Eric,

On Mon, Nov 07, 2016 at 10:44:44AM +0100, Eric Botcazou wrote:
> Tested on PowerPC64/Linux, OK for the mainline?
> 
> 
>* config/rs6000/rs6000.c (rs6000_emit_move): Also use a TOC reference
> after forcing to constant memory when the code model is medium.

Sorry I lost track of this patch again.

After a lot of staring it does seem okay.  Approved for trunk.  Please
watch out for fallout though.

Thanks,


Segher


> --- config/rs6000/rs6000.c(revision 241856)
> +++ config/rs6000/rs6000.c(working copy)
> @@ -10673,10 +10673,7 @@ rs6000_emit_move (rtx dest, rtx source,
>  
> if (TARGET_TOC
> && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
> -   && constant_pool_expr_p (XEXP (operands[1], 0))
> -   && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (
> - get_pool_constant (XEXP (operands[1], 0)),
> - get_pool_mode (XEXP (operands[1], 0
> +   && use_toc_relative_ref (XEXP (operands[1], 0), mode))
>   {
> rtx tocref = create_TOC_reference (XEXP (operands[1], 0),
>operands[0]);

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Eric Botcazou]

> Now you don't need to have a special pool to call create_TOC_reference, you
> can call it for regular TOC references as well, as done a few lines above:
> 
>   /* If this is a SYMBOL_REF that refers to a constant pool entry,
>and we have put it in the TOC, we just need to make a TOC-relative
>reference to it.  */
>   if (TARGET_TOC
> && GET_CODE (operands[1]) == SYMBOL_REF
> && use_toc_relative_ref (operands[1], mode))
>   operands[1] = create_TOC_reference (operands[1], operands[0]);
> 
> So the attached patch does it there too.
> 
> Tested on PowerPC64/Linux (LRA) and VxWorks (reload), OK for the mainline?

Revised version attached, with Pmode formally changed to mode (but mode == 
Pmode here so no functional change whatsoever).

Tested on PowerPC64/Linux, OK for the mainline?


   * config/rs6000/rs6000.c (rs6000_emit_move): Also use a TOC reference
after forcing to constant memory when the code model is medium.

-- 
Eric BotcazouIndex: config/rs6000/rs6000.c
===
--- config/rs6000/rs6000.c	(revision 241856)
+++ config/rs6000/rs6000.c	(working copy)
@@ -10673,10 +10673,7 @@ rs6000_emit_move (rtx dest, rtx source,
 
 	  if (TARGET_TOC
 	  && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
-	  && constant_pool_expr_p (XEXP (operands[1], 0))
-	  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (
-			get_pool_constant (XEXP (operands[1], 0)),
-			get_pool_mode (XEXP (operands[1], 0
+	  && use_toc_relative_ref (XEXP (operands[1], 0), mode))
 	{
 	  rtx tocref = create_TOC_reference (XEXP (operands[1], 0),
 		 operands[0]);

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Eric Botcazou]

> I don't fully understand what you mean.  This code was created for
> PR65810, if that helps?

OK, let's turn it into "mode" then, this doesn't change anything.

-- 
Eric Botcazou

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Segher Boessenkool]

On Tue, Oct 18, 2016 at 08:37:47PM +0200, Eric Botcazou wrote:
> > We need to pass the mode of the actual datum we would put in the TOC to
> > the use_toc_relative_ref function, not the mode of its address.
> 
> Right, but this mode is not "mode", the TOC contains only Pmode entries if 
> the 
> special constant pool is excluded.

I don't fully understand what you mean.  This code was created for
PR65810, if that helps?


Segher

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Eric Botcazou]

> We need to pass the mode of the actual datum we would put in the TOC to
> the use_toc_relative_ref function, not the mode of its address.

Right, but this mode is not "mode", the TOC contains only Pmode entries if the 
special constant pool is excluded.

-- 
Eric Botcazou

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Segher Boessenkool]

On Tue, Oct 18, 2016 at 01:09:24PM +0200, Eric Botcazou wrote:
> > > No, "mode" is the mode of the MEM, not that of the SYMBOL_REF.
> > 
> > I still don't see it, could you explain a bit more?
> 
> MODE is the mode of operands[1] before:
> 
> operands[1] = force_const_mem (mode, operands[1]);
> 
> and after.  But the test is on the address of the MEM, not on the MEM itself:
> 
> if (TARGET_TOC
> && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
> && use_toc_relative_ref (XEXP (operands[1], 0), Pmode))
> 
> because it's the mode of SYMBOL_REF we are interesting in (and 
> force_const_mem 
> guarantees that it's Pmode).

We need to pass the mode of the actual datum we would put in the TOC to
the use_toc_relative_ref function, not the mode of its address.

I must be missing something...


Segher

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Eric Botcazou]

> > No, "mode" is the mode of the MEM, not that of the SYMBOL_REF.
> 
> I still don't see it, could you explain a bit more?

MODE is the mode of operands[1] before:

  operands[1] = force_const_mem (mode, operands[1]);

and after.  But the test is on the address of the MEM, not on the MEM itself:

  if (TARGET_TOC
  && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
  && use_toc_relative_ref (XEXP (operands[1], 0), Pmode))

because it's the mode of SYMBOL_REF we are interesting in (and force_const_mem 
guarantees that it's Pmode).  IOW you could theoretically have mode == SImode 
and we would still need to pass Pmode to use_toc_relative_ref (of course the 
whole thing is guarded with mode == Pmode so that's a little artificial).

-- 
Eric Botcazou

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Segher Boessenkool]

[ sorry for losing track of this patch ]

On Sun, Oct 09, 2016 at 10:32:51AM +0200, Eric Botcazou wrote:
> > Use "mode" instead of "Pmode" here?
> 
> No, "mode" is the mode of the MEM, not that of the SYMBOL_REF.

I still don't see it, could you explain a bit more?


Segher

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Eric Botcazou]

> Why does this not fail on darwin?  It can reach this code afaics, and
> it has ASM_OUTPUT_SPECIAL_POOL_ENTRY_P always 0.

Probably because Darwin doesn't use the TOC at all.

> Use "mode" instead of "Pmode" here?

No, "mode" is the mode of the MEM, not that of the SYMBOL_REF.

-- 
Eric Botcazou

Re: [rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Segher Boessenkool]

Hi!

On Sat, Oct 08, 2016 at 07:53:42PM +0200, Eric Botcazou wrote:
> this fixes reload failures in 64-bit mode with -mcmodel=medium for targets 
> that define ASM_OUTPUT_SPECIAL_POOL_ENTRY_P to 0; that's the case for the 
> VxWorks port we'll submit because of the compatibility with the system 
> linker, 
> which rejects .got sections in object files.
> 
> When reload ends up calling rs6000_emit_move on a register on the LHS and 
> (const (symbol_ref) (const_int)) on the RHS, the function invokes:
> 
> operands[1] = force_const_mem (mode, operands[1]);
> 
> creating (mem (symbol_ref)), which then needs to be legitimized on TARGET_TOC 
> targets.  That's done by the immediately following block:
> 
> if (TARGET_TOC
> && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
> && constant_pool_expr_p (XEXP (operands[1], 0))
> && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (
>   get_pool_constant (XEXP (operands[1], 0)),
>   get_pool_mode (XEXP (operands[1], 0
>   {
> rtx tocref = create_TOC_reference (XEXP (operands[1], 0),
>operands[0]);
> operands[1] = gen_const_mem (mode, tocref);
> set_mem_alias_set (operands[1], get_TOC_alias_set ());
>   }
> 
> on most targets, but is disabled if ASM_OUTPUT_SPECIAL_POOL_ENTRY_P is 0.

Why does this not fail on darwin?  It can reach this code afaics, and
it has ASM_OUTPUT_SPECIAL_POOL_ENTRY_P always 0.

> Now you don't need to have a special pool to call create_TOC_reference, you 
> can call it for regular TOC references as well, as done a few lines above:
> 
>   /* If this is a SYMBOL_REF that refers to a constant pool entry,
>and we have put it in the TOC, we just need to make a TOC-relative
>reference to it.  */
>   if (TARGET_TOC
> && GET_CODE (operands[1]) == SYMBOL_REF
> && use_toc_relative_ref (operands[1], mode))
>   operands[1] = create_TOC_reference (operands[1], operands[0]);
> 
> So the attached patch does it there too.
> 
> Tested on PowerPC64/Linux (LRA) and VxWorks (reload), OK for the mainline?

Not sure yet, will get back to you.  One comment in the meantime...

> --- config/rs6000/rs6000.c(revision 240888)
> +++ config/rs6000/rs6000.c(working copy)
> @@ -10590,10 +10590,7 @@ rs6000_emit_move (rtx dest, rtx source,
>  
> if (TARGET_TOC
> && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
> -   && constant_pool_expr_p (XEXP (operands[1], 0))
> -   && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (
> - get_pool_constant (XEXP (operands[1], 0)),
> - get_pool_mode (XEXP (operands[1], 0
> +   && use_toc_relative_ref (XEXP (operands[1], 0), Pmode))

Use "mode" instead of "Pmode" here?


Segher

[rs6000] Fix reload failures in 64-bit mode with no special constant pool

[Eric Botcazou]

Hi,

this fixes reload failures in 64-bit mode with -mcmodel=medium for targets 
that define ASM_OUTPUT_SPECIAL_POOL_ENTRY_P to 0; that's the case for the 
VxWorks port we'll submit because of the compatibility with the system linker, 
which rejects .got sections in object files.

When reload ends up calling rs6000_emit_move on a register on the LHS and 
(const (symbol_ref) (const_int)) on the RHS, the function invokes:

  operands[1] = force_const_mem (mode, operands[1]);

creating (mem (symbol_ref)), which then needs to be legitimized on TARGET_TOC 
targets.  That's done by the immediately following block:

  if (TARGET_TOC
  && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
  && constant_pool_expr_p (XEXP (operands[1], 0))
  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (
get_pool_constant (XEXP (operands[1], 0)),
get_pool_mode (XEXP (operands[1], 0
{
  rtx tocref = create_TOC_reference (XEXP (operands[1], 0),
 operands[0]);
  operands[1] = gen_const_mem (mode, tocref);
  set_mem_alias_set (operands[1], get_TOC_alias_set ());
}

on most targets, but is disabled if ASM_OUTPUT_SPECIAL_POOL_ENTRY_P is 0.

Now you don't need to have a special pool to call create_TOC_reference, you 
can call it for regular TOC references as well, as done a few lines above:

  /* If this is a SYMBOL_REF that refers to a constant pool entry,
 and we have put it in the TOC, we just need to make a TOC-relative
 reference to it.  */
  if (TARGET_TOC
  && GET_CODE (operands[1]) == SYMBOL_REF
  && use_toc_relative_ref (operands[1], mode))
operands[1] = create_TOC_reference (operands[1], operands[0]);

So the attached patch does it there too.

Tested on PowerPC64/Linux (LRA) and VxWorks (reload), OK for the mainline?


2016-10-08  Eric Botcazou  

* config/rs6000/rs6000.c (rs6000_emit_move): Also use a TOC reference
after forcing to constant memory when the code model is medium.

-- 
Eric BotcazouIndex: config/rs6000/rs6000.c
===
--- config/rs6000/rs6000.c	(revision 240888)
+++ config/rs6000/rs6000.c	(working copy)
@@ -10590,10 +10590,7 @@ rs6000_emit_move (rtx dest, rtx source,
 
 	  if (TARGET_TOC
 	  && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
-	  && constant_pool_expr_p (XEXP (operands[1], 0))
-	  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (
-			get_pool_constant (XEXP (operands[1], 0)),
-			get_pool_mode (XEXP (operands[1], 0
+	  && use_toc_relative_ref (XEXP (operands[1], 0), Pmode))
 	{
 	  rtx tocref = create_TOC_reference (XEXP (operands[1], 0),
 		 operands[0]);