[PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Uros Bizjak]

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

Hello!

Attached patch removes reload_in_progress checks for x86 (LRA enabled)
target. AFAICS, reload_in_progress is never set during the
compilation, a watchpoint on this variable didn't trigger for a couple
of complex compilations.

2015-04-19  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.c (set_pic_reg_ever_live): Remove.
    (legitimize_pic_address): Do not call set_pic_reg_ever_live.
    (legitimize_tls_address): Ditto.
    (ix86_expand_move): Ditto.
    (ix86_expand_binary_operator): Remove reload_in_progress checks.
    (ix86_expand_unary_operator): Ditto.
    * config/i386/predicates.md (index_register_operand): Ditto.

Patch was bootstrapped on x86_64-linux-gnu and regression tested for
x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic.

The patch also changes Darwin specific code that can't be tested
properly on linux. Instead of leaving a reload_in_progress_check
theere, I'd ask someone to bootstrap and regression test the patch on
Darwin target.

I'll wait for the Darwin regression test results (and possible
comments) before committing the patch.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 4265 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 222194)
+++ config/i386/i386.c	(working copy)
@@ -13576,15 +13576,6 @@ ix86_GOT_alias_set (void)
   return set;
 }
 
-/* Set regs_ever_live for PIC base address register
-   to true if required.  */
-static void
-set_pic_reg_ever_live ()
-{
-  if (reload_in_progress)
-    df_set_regs_ever_live (REGNO (pic_offset_table_rtx), true);
-}
-
 /* Return a legitimate reference for ORIG (an address) using the
    register REG.  If REG is 0, a new pseudo is generated.
 
@@ -13635,7 +13626,6 @@ legitimize_pic_address (rtx orig, rtx reg)
       /* This symbol may be referenced via a displacement from the PIC
 	 base address (@GOTOFF).  */
 
-      set_pic_reg_ever_live ();
       if (GET_CODE (addr) == CONST)
 	addr = XEXP (addr, 0);
       if (GET_CODE (addr) == PLUS)
@@ -13667,7 +13657,6 @@ legitimize_pic_address (rtx orig, rtx reg)
       /* This symbol may be referenced via a displacement from the PIC
 	 base address (@GOTOFF).  */
 
-      set_pic_reg_ever_live ();
       if (GET_CODE (addr) == CONST)
 	addr = XEXP (addr, 0);
       if (GET_CODE (addr) == PLUS)
@@ -13728,7 +13717,6 @@ legitimize_pic_address (rtx orig, rtx reg)
 	  /* This symbol must be referenced via a load from the
 	     Global Offset Table (@GOT).  */
 
-	  set_pic_reg_ever_live ();
 	  new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr), UNSPEC_GOT);
 	  new_rtx = gen_rtx_CONST (Pmode, new_rtx);
 	  if (TARGET_64BIT)
@@ -13780,7 +13768,6 @@ legitimize_pic_address (rtx orig, rtx reg)
 	    {
 	      if (!TARGET_64BIT)
 		{
-		  set_pic_reg_ever_live ();
 		  new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, op0),
 					    UNSPEC_GOTOFF);
 		  new_rtx = gen_rtx_PLUS (Pmode, new_rtx, op1);
@@ -14082,7 +14069,6 @@ legitimize_tls_address (rtx x, enum tls_model mode
 	}
       else if (flag_pic)
 	{
-	  set_pic_reg_ever_live ();
 	  pic = pic_offset_table_rtx;
 	  type = TARGET_ANY_GNU_TLS ? UNSPEC_GOTNTPOFF : UNSPEC_GOTTPOFF;
 	}
@@ -17306,10 +17292,8 @@ ix86_expand_move (machine_mode mode, rtx operands[
 	  /* dynamic-no-pic */
 	  if (MACHOPIC_INDIRECT)
 	    {
-	      rtx temp = ((reload_in_progress
-			   || ((op0 && REG_P (op0))
-			       && mode == Pmode))
-			  ? op0 : gen_reg_rtx (Pmode));
+	      rtx temp = (op0 && REG_P (op0) && mode == Pmode)
+			 ? op0 : gen_reg_rtx (Pmode));
 	      op1 = machopic_indirect_data_reference (op1, temp);
 	      if (MACHOPIC_PURE)
 		op1 = machopic_legitimize_pic_address (op1, mode,
@@ -17957,17 +17941,11 @@ ix86_expand_binary_operator (enum rtx_code code, m
  /* Emit the instruction.  */
 
   op = gen_rtx_SET (VOIDmode, dst, gen_rtx_fmt_ee (code, mode, src1, src2));
-  if (reload_in_progress)
+
+  if (reload_completed
+      && code == PLUS
+      && !rtx_equal_p (dst, src1))
     {
-      /* Reload doesn't know about the flags register, and doesn't know that
-         it doesn't want to clobber it.  We can only do this with PLUS.  */
-      gcc_assert (code == PLUS);
-      emit_insn (op);
-    }
-  else if (reload_completed
-	   && code == PLUS
-	   && !rtx_equal_p (dst, src1))
-    {
       /* This is going to be an LEA; avoid splitting it later.  */
       emit_insn (op);
     }
@@ -18130,13 +18108,9 @@ ix86_expand_unary_operator (enum rtx_code code, ma
   /* Emit the instruction.  */
 
   op = gen_rtx_SET (VOIDmode, dst, gen_rtx_fmt_e (code, mode, src));
-  if (reload_in_progress || code == NOT)
-    {
-      /* Reload doesn't know about the flags register, and doesn't know that
-         it doesn't want to clobber it.  */
-      gcc_assert (code == NOT);
-      emit_insn (op);
-    }
+
+  if (code == NOT)
+    emit_insn (op);
   else
     {
       clob = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (CCmode, FLAGS_REG));
Index: config/i386/predicates.md
===================================================================
--- config/i386/predicates.md	(revision 222194)
+++ config/i386/predicates.md	(working copy)
@@ -577,7 +577,7 @@
 {
   if (GET_CODE (op) == SUBREG)
     op = SUBREG_REG (op);
-  if (reload_in_progress || reload_completed)
+  if (reload_completed)
     return REG_OK_FOR_INDEX_STRICT_P (op);
   else
     return REG_OK_FOR_INDEX_NONSTRICT_P (op);

Re: [PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Dominique d'Humières]

With the patch bootstrapping x86_64-apple-darwin14 was broken due to a typo:

../../work/gcc/config/i386/i386.c: In function 'void ix86_expand_move(machine_mode, rtx_def**)':
../../work/gcc/config/i386/i386.c:17296:32: error: expected ',' or ';' before ')' token
     ? op0 : gen_reg_rtx (Pmode));
                                ^

After removing the extra right parenthesis, I am now at stage 2.

Dominique

> Le 19 avr. 2015 à 20:35, Uros Bizjak <ubizjak@gmail.com> a écrit :
> 
> Hello!
> 
> Attached patch removes reload_in_progress checks for x86 (LRA enabled)
> target. AFAICS, reload_in_progress is never set during the
> compilation, a watchpoint on this variable didn't trigger for a couple
> of complex compilations.
> 
> 2015-04-19  Uros Bizjak  <ubizjak@gmail.com>
> 
>    * config/i386/i386.c (set_pic_reg_ever_live): Remove.
>    (legitimize_pic_address): Do not call set_pic_reg_ever_live.
>    (legitimize_tls_address): Ditto.
>    (ix86_expand_move): Ditto.
>    (ix86_expand_binary_operator): Remove reload_in_progress checks.
>    (ix86_expand_unary_operator): Ditto.
>    * config/i386/predicates.md (index_register_operand): Ditto.
> 
> Patch was bootstrapped on x86_64-linux-gnu and regression tested for
> x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic.
> 
> The patch also changes Darwin specific code that can't be tested
> properly on linux. Instead of leaving a reload_in_progress_check
> theere, I'd ask someone to bootstrap and regression test the patch on
> Darwin target.
> 
> I'll wait for the Darwin regression test results (and possible
> comments) before committing the patch.
> 
> Uros.
> <p.diff.txt>

Re: [PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Dominique d'Humières]

After having fixed the typo, regtesting went without regression.

Dominique

> Le 19 avr. 2015 à 20:35, Uros Bizjak <ubizjak@gmail.com> a écrit :
> 
> Hello!
> 
> Attached patch removes reload_in_progress checks for x86 (LRA enabled)
> target. AFAICS, reload_in_progress is never set during the
> compilation, a watchpoint on this variable didn't trigger for a couple
> of complex compilations.
> 
> 2015-04-19  Uros Bizjak  <ubizjak@gmail.com>
> 
>    * config/i386/i386.c (set_pic_reg_ever_live): Remove.
>    (legitimize_pic_address): Do not call set_pic_reg_ever_live.
>    (legitimize_tls_address): Ditto.
>    (ix86_expand_move): Ditto.
>    (ix86_expand_binary_operator): Remove reload_in_progress checks.
>    (ix86_expand_unary_operator): Ditto.
>    * config/i386/predicates.md (index_register_operand): Ditto.
> 
> Patch was bootstrapped on x86_64-linux-gnu and regression tested for
> x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic.
> 
> The patch also changes Darwin specific code that can't be tested
> properly on linux. Instead of leaving a reload_in_progress_check
> theere, I'd ask someone to bootstrap and regression test the patch on
> Darwin target.
> 
> I'll wait for the Darwin regression test results (and possible
> comments) before committing the patch.
> 
> Uros.
> <p.diff.txt>

Re: [PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Iain Sandoe]

On 20 Apr 2015, at 10:47, Dominique d'Humières wrote:

> After having fixed the typo, regtesting went without regression.

I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far),

NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations.  I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present).

Iain

> 
> Dominique
> 
>> Le 19 avr. 2015 à 20:35, Uros Bizjak <ubizjak@gmail.com> a écrit :
>> 
>> Hello!
>> 
>> Attached patch removes reload_in_progress checks for x86 (LRA enabled)
>> target. AFAICS, reload_in_progress is never set during the
>> compilation, a watchpoint on this variable didn't trigger for a couple
>> of complex compilations.
>> 
>> 2015-04-19  Uros Bizjak  <ubizjak@gmail.com>
>> 
>>   * config/i386/i386.c (set_pic_reg_ever_live): Remove.
>>   (legitimize_pic_address): Do not call set_pic_reg_ever_live.
>>   (legitimize_tls_address): Ditto.
>>   (ix86_expand_move): Ditto.
>>   (ix86_expand_binary_operator): Remove reload_in_progress checks.
>>   (ix86_expand_unary_operator): Ditto.
>>   * config/i386/predicates.md (index_register_operand): Ditto.
>> 
>> Patch was bootstrapped on x86_64-linux-gnu and regression tested for
>> x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic.
>> 
>> The patch also changes Darwin specific code that can't be tested
>> properly on linux. Instead of leaving a reload_in_progress_check
>> theere, I'd ask someone to bootstrap and regression test the patch on
>> Darwin target.
>> 
>> I'll wait for the Darwin regression test results (and possible
>> comments) before committing the patch.
>> 
>> Uros.
>> <p.diff.txt>
> 

Re: [PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Uros Bizjak]

On Mon, Apr 20, 2015 at 12:00 PM, Iain Sandoe <iain@codesourcery.com> wrote:

>> After having fixed the typo, regtesting went without regression.
>
> I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far),
>
> NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations.  I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present).

Probably a better way is to include "targetm.lra_p ()" into the check.

Uros.

Re: [PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Mike Stump]

On Apr 20, 2015, at 3:16 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Apr 20, 2015 at 12:00 PM, Iain Sandoe <iain@codesourcery.com> wrote:
> 
>>> After having fixed the typo, regtesting went without regression.
>> 
>> I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far),
>> 
>> NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations.  I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present).
> 
> Probably a better way is to include "targetm.lra_p ()" into the check.

Only if you discount asking the nice rs6000/powerpc people to lra the entire port, and by that, I mean, remove the non-lra code.  :-)

Re: [PATCH, i386, Darwin RFT]: Remove reload_in_progress checks

[Uros Bizjak]

On Mon, Apr 20, 2015 at 12:00 PM, Iain Sandoe <iain@codesourcery.com> wrote:
>
> On 20 Apr 2015, at 10:47, Dominique d'Humières wrote:
>
>> After having fixed the typo, regtesting went without regression.
>
> I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far),
>
> NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations.  I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present).

Thanks!

I have committed the fixed patch to the SVN as r222246.

Uros.