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

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

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]

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  <ebotcazou@adacore.com>

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

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 681 bytes --]

Index: 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]);

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

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]

[ 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]

> > 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]

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]

> 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 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]

> 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

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 980 bytes --]

> 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 Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 680 bytes --]

Index: 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

[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]);