[PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

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

Hi,

The patch enables EBX in RA for x86 32bits PIC mode.
It was discussed here: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02513.html
Now there is working version with good performance and stability level
- it could be a solid first step of EBX enabling.

Bootstrap and make check passed.
There are several changes in "-m32" make check.

New pass:
gcc.target/i386/pr57003.c - before patch there was not enough registers to PASS

New fails:

gcc.target/i386/pic-1.c (test for errors, line 12) - now there are no
errors as we can do asm insertions with EBX
gcc.target/i386/pr23098.c scan-assembler-not .LC[0-9] - potential
performance opportunity using constant immediate
gcc.target/i386/pr55458.c (test for errors, line 10) - now there are
no errors as there enough registers

Performance on Silvermont (for the group of 3 patches):

at -Ofast -flto -funroll-loops -fPIC
EEMBC benchs:
 +6% CoreMark
 +3% Denmark
Individual test gained up to +25%

SPEC2000int is almost unchanged, SPEC2000fp +1,7%:

at -O2 -fPIC
EEMBC benchs:
 +2% CoreMark
 +5% Denmark
Individual test gained up to +25%

SPEC2000int and SPEC2000fp are almost unchanged:

ChangeLog:

2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
            Vladimir Makarov  <vmakarov@redhat.com>
        * doc/tm.texi.in (TARGET_USE_PSEUDO_PIC_REG, TARGET_INIT_PIC_REG):
        Document.
        * doc/tm.texi: Regenerate.
        * gcc/function.c (assign_parms): Generate pseudo register for PIC.
        * gcc/init-regs.c (initialize_uninitialized_regs): Ignor pseudo PIC
        register.
        * gcc/ira-color.c (color_pass): Add check on pseudo register.
        * gcc/ira-emit.c (change_loop): Don't create copies for PIC pseudo
        register.
        * gcc/ira.c (split_live_ranges_for_shrink_wrap): Add check on pseudo
        register.
        (ira): Add target specific PIC register initialization.
        (do_reload): Keep PIC pseudo register.
        * gcc/lra-assigns.c (spill_for): Add checks on pseudo register.
        * gcc/lra-constraints.c (contains_symbol_ref_p): New.
        (lra_constraints): Enable lra risky transformations when PIC is pseudo
        register.
        * gcc/shrink-wrap.c (try_shrink_wrapping): Add check on pseudo register.
        * gcc/target.def (use_pseudo_pic_reg): New.
        (init_pic_reg): New.

[-- Attachment #2: enabling_ebx_general.patch --]
[-- Type: application/octet-stream, Size: 10633 bytes --]

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 396909f..0dd9b79 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -3909,6 +3909,16 @@ If @code{TARGET_FUNCTION_INCOMING_ARG} is not defined,
 @code{TARGET_FUNCTION_ARG} serves both purposes.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_USE_PSEUDO_PIC_REG (void)
+This hook should return 1 in case pseudo register should be created
+for pic_offset_table_rtx during function expand.
+@end deftypefn
+
+@deftypefn {Target Hook} void TARGET_INIT_PIC_REG (void)
+Perform a target dependent initialization of pic_offset_table_rtx.
+This hook is called at the start of register allocation.
+@end deftypefn
+
 @deftypefn {Target Hook} int TARGET_ARG_PARTIAL_BYTES (cumulative_args_t @var{cum}, enum machine_mode @var{mode}, tree @var{type}, bool @var{named})
 This target hook returns the number of bytes at the beginning of an
 argument that must be put in registers.  The value must be zero for
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 798c1aa..d6ee52a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3355,6 +3355,10 @@ the stack.
 
 @hook TARGET_FUNCTION_INCOMING_ARG
 
+@hook TARGET_USE_PSEUDO_PIC_REG
+
+@hook TARGET_INIT_PIC_REG
+
 @hook TARGET_ARG_PARTIAL_BYTES
 
 @hook TARGET_PASS_BY_REFERENCE
diff --git a/gcc/function.c b/gcc/function.c
index ac50f4a..cd7e42e 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3459,6 +3459,11 @@ assign_parms (tree fndecl)
 
   fnargs.release ();
 
+  /* Initialize pic_offset_table_rtx with a pseudo register
+     if required.  */
+  if (targetm.use_pseudo_pic_reg ())
+    pic_offset_table_rtx = gen_reg_rtx (Pmode);
+
   /* Output all parameter conversion instructions (possibly including calls)
      now that all parameters have been copied out of hard registers.  */
   emit_insn (all.first_conversion_insn);
diff --git a/gcc/init-regs.c b/gcc/init-regs.c
index 91b123d..bf83e51 100644
--- a/gcc/init-regs.c
+++ b/gcc/init-regs.c
@@ -80,6 +80,11 @@ initialize_uninitialized_regs (void)
 	      if (regno < FIRST_PSEUDO_REGISTER)
 		continue;
 
+	      /* Ignore pseudo PIC register.  */
+	      if (pic_offset_table_rtx
+		  && regno == REGNO (pic_offset_table_rtx))
+		continue;
+
 	      /* Do not generate multiple moves for the same regno.
 		 This is common for sequences of subreg operations.
 		 They would be deleted during combine but there is no
diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 6846567..26b8ffe 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -3239,9 +3239,11 @@ color_pass (ira_loop_tree_node_t loop_tree_node)
 	  ira_assert (ALLOCNO_CLASS (subloop_allocno) == rclass);
 	  ira_assert (bitmap_bit_p (subloop_node->all_allocnos,
 				    ALLOCNO_NUM (subloop_allocno)));
-	  if ((flag_ira_region == IRA_REGION_MIXED)
-	      && (loop_tree_node->reg_pressure[pclass]
-		  <= ira_class_hard_regs_num[pclass]))
+	  if ((flag_ira_region == IRA_REGION_MIXED
+	       && (loop_tree_node->reg_pressure[pclass]
+		   <= ira_class_hard_regs_num[pclass]))
+	      || (pic_offset_table_rtx != NULL
+		  && regno == (int) REGNO (pic_offset_table_rtx)))
 	    {
 	      if (! ALLOCNO_ASSIGNED_P (subloop_allocno))
 		{
diff --git a/gcc/ira-emit.c b/gcc/ira-emit.c
index a3bf41e..676ee1a 100644
--- a/gcc/ira-emit.c
+++ b/gcc/ira-emit.c
@@ -620,7 +620,10 @@ change_loop (ira_loop_tree_node_t node)
 		  /* don't create copies because reload can spill an
 		     allocno set by copy although the allocno will not
 		     get memory slot.  */
-		  || ira_equiv_no_lvalue_p (regno)))
+		  || ira_equiv_no_lvalue_p (regno)
+		  || (pic_offset_table_rtx != NULL
+		      && (ALLOCNO_REGNO (allocno)
+			  == (int) REGNO (pic_offset_table_rtx)))))
 	    continue;
 	  original_reg = allocno_emit_reg (allocno);
 	  if (parent_allocno == NULL
diff --git a/gcc/ira.c b/gcc/ira.c
index f377f7d..ae83aa5 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -4887,7 +4887,7 @@ split_live_ranges_for_shrink_wrap (void)
   FOR_BB_INSNS (first, insn)
     {
       rtx dest = interesting_dest_for_shprep (insn, call_dom);
-      if (!dest)
+      if (!dest || dest == pic_offset_table_rtx)
 	continue;
 
       rtx newreg = NULL_RTX;
@@ -5039,6 +5039,9 @@ ira (FILE *f)
   bool saved_flag_caller_saves = flag_caller_saves;
   enum ira_region saved_flag_ira_region = flag_ira_region;
 
+  /* Perform target specific PIC register initialization.  */
+  targetm.init_pic_reg ();
+
   ira_conflicts_p = optimize > 0;
 
   ira_use_lra_p = targetm.lra_p ();
@@ -5290,10 +5293,18 @@ do_reload (void)
 {
   basic_block bb;
   bool need_dce;
+  unsigned pic_offset_table_regno = INVALID_REGNUM;
 
   if (flag_ira_verbose < 10)
     ira_dump_file = dump_file;
 
+  /* If pic_offset_table_rtx is a pseudo register, then keep it so
+     after reload to avoid possible wrong usages of hard reg assigned
+     to it.  */
+  if (pic_offset_table_rtx
+      && REGNO (pic_offset_table_rtx) >= FIRST_PSEUDO_REGISTER)
+    pic_offset_table_regno = REGNO (pic_offset_table_rtx);
+
   timevar_push (TV_RELOAD);
   if (ira_use_lra_p)
     {
@@ -5398,6 +5409,9 @@ do_reload (void)
       inform (DECL_SOURCE_LOCATION (decl), "for %qD", decl);
     }
 
+  if (pic_offset_table_regno != INVALID_REGNUM)
+    pic_offset_table_rtx = gen_rtx_REG (Pmode, pic_offset_table_regno);
+
   timevar_pop (TV_IRA);
 }
 \f
diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
index c7164cd..99ae00d 100644
--- a/gcc/lra-assigns.c
+++ b/gcc/lra-assigns.c
@@ -879,11 +879,13 @@ spill_for (int regno, bitmap spilled_pseudo_bitmap, bool first_p)
 	}
       /* Spill pseudos.	 */
       EXECUTE_IF_SET_IN_BITMAP (&spill_pseudos_bitmap, 0, spill_regno, bi)
-	if ((int) spill_regno >= lra_constraint_new_regno_start
-	    && ! bitmap_bit_p (&lra_inheritance_pseudos, spill_regno)
-	    && ! bitmap_bit_p (&lra_split_regs, spill_regno)
-	    && ! bitmap_bit_p (&lra_subreg_reload_pseudos, spill_regno)
-	    && ! bitmap_bit_p (&lra_optional_reload_pseudos, spill_regno))
+	if ((pic_offset_table_rtx != NULL
+	     && spill_regno == REGNO (pic_offset_table_rtx))
+	    || ((int) spill_regno >= lra_constraint_new_regno_start
+		&& ! bitmap_bit_p (&lra_inheritance_pseudos, spill_regno)
+		&& ! bitmap_bit_p (&lra_split_regs, spill_regno)
+		&& ! bitmap_bit_p (&lra_subreg_reload_pseudos, spill_regno)
+		&& ! bitmap_bit_p (&lra_optional_reload_pseudos, spill_regno)))
 	  goto fail;
       insn_pseudos_num = 0;
       if (lra_dump_file != NULL)
@@ -1053,9 +1055,15 @@ setup_live_pseudos_and_spill_after_risky_transforms (bitmap
       return;
     }
   for (n = 0, i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
-    if (reg_renumber[i] >= 0 && lra_reg_info[i].nrefs > 0)
+    if ((pic_offset_table_rtx == NULL_RTX
+	 || i != (int) REGNO (pic_offset_table_rtx))
+	&& reg_renumber[i] >= 0 && lra_reg_info[i].nrefs > 0)
       sorted_pseudos[n++] = i;
   qsort (sorted_pseudos, n, sizeof (int), pseudo_compare_func);
+  if (pic_offset_table_rtx != NULL_RTX
+      && (regno = REGNO (pic_offset_table_rtx)) >= FIRST_PSEUDO_REGISTER
+      && reg_renumber[regno] >= 0 && lra_reg_info[regno].nrefs > 0)
+    sorted_pseudos[n++] = regno;
   for (i = n - 1; i >= 0; i--)
     {
       regno = sorted_pseudos[i];
diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 5f68399..977e1db 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -3798,6 +3798,35 @@ contains_reg_p (rtx x, bool hard_reg_p, bool spilled_p)
   return false;
 }
 
+/* Return true if X contains a symbol reg.  */
+static bool
+contains_symbol_ref_p (rtx x)
+{
+  int i, j;
+  const char *fmt;
+  enum rtx_code code;
+
+  code = GET_CODE (x);
+  if (code == SYMBOL_REF)
+    return true;
+  fmt = GET_RTX_FORMAT (code);
+  for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
+    {
+      if (fmt[i] == 'e')
+	{
+	  if (contains_symbol_ref_p (XEXP (x, i)))
+	    return true;
+	}
+      else if (fmt[i] == 'E')
+	{
+	  for (j = XVECLEN (x, i) - 1; j >= 0; j--)
+	    if (contains_symbol_ref_p (XVECEXP (x, i, j)))
+	      return true;
+	}
+    }
+  return false;
+}
+
 /* Process all regs in location *LOC and change them on equivalent
    substitution.  Return true if any change was done.  */
 static bool
@@ -4020,7 +4049,11 @@ lra_constraints (bool first_p)
       ("Maximum number of LRA constraint passes is achieved (%d)\n",
        LRA_MAX_CONSTRAINT_ITERATION_NUMBER);
   changed_p = false;
-  lra_risky_transformations_p = false;
+  if (pic_offset_table_rtx
+      && REGNO (pic_offset_table_rtx) >= FIRST_PSEUDO_REGISTER)
+    lra_risky_transformations_p = true;
+  else
+    lra_risky_transformations_p = false;
   new_insn_uid_start = get_max_uid ();
   new_regno_start = first_p ? lra_constraint_new_regno_start : max_reg_num ();
   /* Mark used hard regs for target stack size calulations.  */
@@ -4088,7 +4121,12 @@ lra_constraints (bool first_p)
 		   paradoxical subregs.  */
 		|| (MEM_P (x)
 		    && (GET_MODE_SIZE (lra_reg_info[i].biggest_mode)
-			> GET_MODE_SIZE (GET_MODE (x)))))
+			> GET_MODE_SIZE (GET_MODE (x))))
+		|| (pic_offset_table_rtx
+		    && ((CONST_POOL_OK_P (PSEUDO_REGNO_MODE (i), x)
+			 && (targetm.preferred_reload_class
+			     (x, lra_get_allocno_class (i)) == NO_REGS))
+			|| contains_symbol_ref_p (x))))
 	      ira_reg_equiv[i].defined_p = false;
 	    if (contains_reg_p (x, false, true))
 	      ira_reg_equiv[i].profitable_p = false;
diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index fd24135..e1ecff7 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c
@@ -495,7 +495,8 @@ try_shrink_wrapping (edge *entry_edge, edge orig_entry_edge,
       if (frame_pointer_needed)
 	add_to_hard_reg_set (&set_up_by_prologue.set, Pmode,
 			     HARD_FRAME_POINTER_REGNUM);
-      if (pic_offset_table_rtx)
+      if (pic_offset_table_rtx
+	  && PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM)
 	add_to_hard_reg_set (&set_up_by_prologue.set, Pmode,
 			     PIC_OFFSET_TABLE_REGNUM);
       if (crtl->drap_reg)
diff --git a/gcc/target.def b/gcc/target.def
index ce11eae..4d90fc2 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4274,6 +4274,20 @@ DEFHOOK
 
 HOOK_VECTOR_END (calls)
 
+DEFHOOK
+(use_pseudo_pic_reg,
+ "This hook should return 1 in case pseudo register should be created\n\
+for pic_offset_table_rtx during function expand.",
+ bool, (void),
+ hook_bool_void_false)
+
+DEFHOOK
+(init_pic_reg,
+ "Perform a target dependent initialization of pic_offset_table_rtx.\n\
+This hook is called at the start of register allocation.",
+ void, (void),
+ hook_void_void)
+
 /* Return the diagnostic message string if conversion from FROMTYPE
    to TOTYPE is not allowed, NULL otherwise.  */
 DEFHOOK

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Vladimir Makarov]

On 2014-10-10 3:42 AM, Evgeny Stupachenko wrote:
> Hi,
>
> The patch enables EBX in RA for x86 32bits PIC mode.
> It was discussed here: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02513.html
> Now there is working version with good performance and stability level
> - it could be a solid first step of EBX enabling.
>
> Bootstrap and make check passed.
> There are several changes in "-m32" make check.
>
> New pass:
> gcc.target/i386/pr57003.c - before patch there was not enough registers to PASS
>
> New fails:
>
> gcc.target/i386/pic-1.c (test for errors, line 12) - now there are no
> errors as we can do asm insertions with EBX
> gcc.target/i386/pr23098.c scan-assembler-not .LC[0-9] - potential
> performance opportunity using constant immediate
> gcc.target/i386/pr55458.c (test for errors, line 10) - now there are
> no errors as there enough registers
>
> Performance on Silvermont (for the group of 3 patches):
>
> at -Ofast -flto -funroll-loops -fPIC
> EEMBC benchs:
>   +6% CoreMark
>   +3% Denmark
> Individual test gained up to +25%
>
> SPEC2000int is almost unchanged, SPEC2000fp +1,7%:
>
> at -O2 -fPIC
> EEMBC benchs:
>   +2% CoreMark
>   +5% Denmark
> Individual test gained up to +25%
>
> SPEC2000int and SPEC2000fp are almost unchanged:
>

Great results!  The changes to RA (all ira* lra* file changes) is ok for 
me.  I'll probably add some comments to these changes on next week.

Thanks.

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jeff Law]

On 10/10/14 01:42, Evgeny Stupachenko wrote:
> Hi,
>
> The patch enables EBX in RA for x86 32bits PIC mode.
> It was discussed here: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02513.html
> Now there is working version with good performance and stability level
> - it could be a solid first step of EBX enabling.
>
> Bootstrap and make check passed.
> There are several changes in "-m32" make check.
>
> New pass:
> gcc.target/i386/pr57003.c - before patch there was not enough registers to PASS
?!?  That doesn't make a lot of sense.  More likely it was Uros's fix 
from yesterday to regcprop which causes this to pass again.

Is it possible you updated your sources between testing runs and as a 
result picked up Uros's fix?

>
> New fails:
>
> gcc.target/i386/pic-1.c (test for errors, line 12) - now there are no
> errors as we can do asm insertions with EBX
I think you should remove the dg-error directive.  That turns this test 
into a simple confirmation that we can use %ebx in an asm even when 
generating PIC code.

Can you add a PR markers to your changelog

	PR target/8340
	PR middle-end/47602
	PR rtl-optimization/55458

Actually I think there is an additional test in 47602.  Can you please 
add it to the suite?  You'll also want to change the state of 47602 to 
RESOLVED/FIXED.




	


> gcc.target/i386/pr23098.c scan-assembler-not .LC[0-9] - potential
> performance opportunity using constant immediate
If you're not going to fix it, then you should xfail it.

> gcc.target/i386/pr55458.c (test for errors, line 10) - now there are
> no errors as there enough registers
Right.  Remove the dg-error and turn this into a test that effective 
verifies that %ebx is no longer fixed when generating PIC code on i686.

With those changes this is OK for the trunk.

jeff

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

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

On Fri, Oct 10, 2014 at 8:03 PM, Jeff Law <law@redhat.com> wrote:
> On 10/10/14 01:42, Evgeny Stupachenko wrote:
>>
>> Hi,
>>
>> The patch enables EBX in RA for x86 32bits PIC mode.
>> It was discussed here:
>> https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02513.html
>> Now there is working version with good performance and stability level
>> - it could be a solid first step of EBX enabling.
>>
>> Bootstrap and make check passed.
>> There are several changes in "-m32" make check.
>>
>> New pass:
>> gcc.target/i386/pr57003.c - before patch there was not enough registers to
>> PASS
>
> ?!?  That doesn't make a lot of sense.  More likely it was Uros's fix from
> yesterday to regcprop which causes this to pass again.
Correct. I've marked it by mistake. The test is flaky and the patch
does not change anything for the test.
>
> Is it possible you updated your sources between testing runs and as a result
> picked up Uros's fix?
>
>>
>> New fails:
>>
>> gcc.target/i386/pic-1.c (test for errors, line 12) - now there are no
>> errors as we can do asm insertions with EBX
>
> I think you should remove the dg-error directive.  That turns this test into
> a simple confirmation that we can use %ebx in an asm even when generating
> PIC code.
>
> Can you add a PR markers to your changelog
>
>         PR target/8340
>         PR middle-end/47602
>         PR rtl-optimization/55458
>
> Actually I think there is an additional test in 47602.  Can you please add
> it to the suite?  You'll also want to change the state of 47602 to
> RESOLVED/FIXED.
>
>
>
>
>
>
>
>> gcc.target/i386/pr23098.c scan-assembler-not .LC[0-9] - potential
>> performance opportunity using constant immediate
>
> If you're not going to fix it, then you should xfail it.
>
>> gcc.target/i386/pr55458.c (test for errors, line 10) - now there are
>> no errors as there enough registers
>
> Right.  Remove the dg-error and turn this into a test that effective
> verifies that %ebx is no longer fixed when generating PIC code on i686.
>
> With those changes this is OK for the trunk.
>
> jeff
>
>

ChangeLog for testsuite:

2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>

        PR target/8340
        PR middle-end/47602
        PR rtl-optimization/55458
        * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
        * gcc.target/i386/pr55458.c: Likewise.
        * gcc.target/i386/pr47602.c: New.
        * gcc.target/i386/pr23098.c: Move to XFAIL.

[-- Attachment #2: enabling_ebx_tests.patch --]
[-- Type: application/octet-stream, Size: 2273 bytes --]

diff --git a/gcc/testsuite/gcc.target/i386/pic-1.c b/gcc/testsuite/gcc.target/i386/pic-1.c
index 9b7da4d..af2424b 100644
--- a/gcc/testsuite/gcc.target/i386/pic-1.c
+++ b/gcc/testsuite/gcc.target/i386/pic-1.c
@@ -5,13 +5,11 @@
 /* { dg-skip-if "No Windows PIC" { *-*-mingw* *-*-cygwin } { "*" } { "" } } */
 /* { dg-options "-fPIC" } */
 
-/* Test verifies that %ebx is no longer fixed when generating PIC code on i686.  */
-
 int foo ()
 {
   static int a;
 
-  __asm__ __volatile__ (
+  __asm__ __volatile__ (  /* { dg-error "PIC register" } */
     "xorl %%ebx, %%ebx\n"
     "movl %%ebx, %0\n"
     : "=m" (a)
diff --git a/gcc/testsuite/gcc.target/i386/pr23098.c b/gcc/testsuite/gcc.target/i386/pr23098.c
index 7f118dc..66ab0e1 100644
--- a/gcc/testsuite/gcc.target/i386/pr23098.c
+++ b/gcc/testsuite/gcc.target/i386/pr23098.c
@@ -1,7 +1,7 @@
 /* PR rtl-optimization/23098 */
 /* { dg-do compile } */
 /* { dg-options "-O2 -fPIC" } */
-/* { dg-final { scan-assembler-not "\.LC\[0-9\]" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-not "\.LC\[0-9\]" { xfail *-*-vxworks* } } } */
 /* { dg-require-effective-target ia32 } */
 /* { dg-require-effective-target fpic } */
 
diff --git a/gcc/testsuite/gcc.target/i386/pr47602.c b/gcc/testsuite/gcc.target/i386/pr47602.c
deleted file mode 100644
index fa5f5bd..0000000
--- a/gcc/testsuite/gcc.target/i386/pr47602.c
+++ /dev/null
@@ -1,13 +0,0 @@
-/* { dg-do compile } */
-/* { dg-require-effective-target ia32 } */
-/* { dg-options "-fPIC" } */
-
-/* Test verifies that %ebx is no longer fixed when generating PIC code on i686.  */
-
-int a, b, c;
-
-void
-foo (void)
-{
-  asm volatile ("movl $0,%%ebx" : : : "ebx");
-}
diff --git a/gcc/testsuite/gcc.target/i386/pr55458.c b/gcc/testsuite/gcc.target/i386/pr55458.c
index 7164ca9..81d85ec 100644
--- a/gcc/testsuite/gcc.target/i386/pr55458.c
+++ b/gcc/testsuite/gcc.target/i386/pr55458.c
@@ -2,12 +2,10 @@
 /* { dg-require-effective-target ia32 } */
 /* { dg-options "-fPIC" } */
 
-/* Test verifies that %ebx is no longer fixed when generating PIC code on i686.  */
-
 int a, b, c;
 
 void
 foo (void)
 {
-  asm volatile ("":"+m" (a), "+m" (b), "+m" (c));
+  asm volatile ("":"+m" (a), "+m" (b), "+m" (c)); /* { dg-error "operand has impossible constraints" } */
 }

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Uros Bizjak]

On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:

> ChangeLog for testsuite:
>
> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>
>         PR target/8340
>         PR middle-end/47602
>         PR rtl-optimization/55458
>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>         * gcc.target/i386/pr55458.c: Likewise.
>         * gcc.target/i386/pr47602.c: New.
>         * gcc.target/i386/pr23098.c: Move to XFAIL.

Reversed patch was attached. Please repost.

Uros.

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jeff Law]

On 10/13/14 08:53, Evgeny Stupachenko wrote:
> ChangeLog for testsuite:
>
> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>
>          PR target/8340
>          PR middle-end/47602
>          PR rtl-optimization/55458
>          * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>          * gcc.target/i386/pr55458.c: Likewise.
>          * gcc.target/i386/pr47602.c: New.
>          * gcc.target/i386/pr23098.c: Move to XFAIL.
>
Looks like you goof'd the patch slightly (reversed).

It's trivial enough that I can see what the correctly ordered patch is 
doing.

OK for the trunk at the same time the rest of the bits go in.

jeff

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

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

Reattached.

On Mon, Oct 13, 2014 at 8:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>
>> ChangeLog for testsuite:
>>
>> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>>
>>         PR target/8340
>>         PR middle-end/47602
>>         PR rtl-optimization/55458
>>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>>         * gcc.target/i386/pr55458.c: Likewise.
>>         * gcc.target/i386/pr47602.c: New.
>>         * gcc.target/i386/pr23098.c: Move to XFAIL.
>
> Reversed patch was attached. Please repost.
>
> Uros.

[-- Attachment #2: enabling_ebx_tests.patch --]
[-- Type: application/octet-stream, Size: 2269 bytes --]

diff --git a/gcc/testsuite/gcc.target/i386/pic-1.c b/gcc/testsuite/gcc.target/i386/pic-1.c
index af2424b..9b7da4d 100644
--- a/gcc/testsuite/gcc.target/i386/pic-1.c
+++ b/gcc/testsuite/gcc.target/i386/pic-1.c
@@ -5,11 +5,13 @@
 /* { dg-skip-if "No Windows PIC" { *-*-mingw* *-*-cygwin } { "*" } { "" } } */
 /* { dg-options "-fPIC" } */
 
+/* Test verifies that %ebx is no longer fixed when generating PIC code on i686.  */
+
 int foo ()
 {
   static int a;
 
-  __asm__ __volatile__ (  /* { dg-error "PIC register" } */
+  __asm__ __volatile__ (
     "xorl %%ebx, %%ebx\n"
     "movl %%ebx, %0\n"
     : "=m" (a)
diff --git a/gcc/testsuite/gcc.target/i386/pr23098.c b/gcc/testsuite/gcc.target/i386/pr23098.c
index 66ab0e1..7f118dc 100644
--- a/gcc/testsuite/gcc.target/i386/pr23098.c
+++ b/gcc/testsuite/gcc.target/i386/pr23098.c
@@ -1,7 +1,7 @@
 /* PR rtl-optimization/23098 */
 /* { dg-do compile } */
 /* { dg-options "-O2 -fPIC" } */
-/* { dg-final { scan-assembler-not "\.LC\[0-9\]" { xfail *-*-vxworks* } } } */
+/* { dg-final { scan-assembler-not "\.LC\[0-9\]" { xfail *-*-* } } } */
 /* { dg-require-effective-target ia32 } */
 /* { dg-require-effective-target fpic } */
 
diff --git a/gcc/testsuite/gcc.target/i386/pr47602.c b/gcc/testsuite/gcc.target/i386/pr47602.c
new file mode 100644
index 0000000..fa5f5bd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47602.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-options "-fPIC" } */
+
+/* Test verifies that %ebx is no longer fixed when generating PIC code on i686.  */
+
+int a, b, c;
+
+void
+foo (void)
+{
+  asm volatile ("movl $0,%%ebx" : : : "ebx");
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr55458.c b/gcc/testsuite/gcc.target/i386/pr55458.c
index 81d85ec..7164ca9 100644
--- a/gcc/testsuite/gcc.target/i386/pr55458.c
+++ b/gcc/testsuite/gcc.target/i386/pr55458.c
@@ -2,10 +2,12 @@
 /* { dg-require-effective-target ia32 } */
 /* { dg-options "-fPIC" } */
 
+/* Test verifies that %ebx is no longer fixed when generating PIC code on i686.  */
+
 int a, b, c;
 
 void
 foo (void)
 {
-  asm volatile ("":"+m" (a), "+m" (b), "+m" (c)); /* { dg-error "operand has impossible constraints" } */
+  asm volatile ("":"+m" (a), "+m" (b), "+m" (c));
 }

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Uros Bizjak]

On Mon, Oct 13, 2014 at 6:32 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> Reattached.
>
> On Mon, Oct 13, 2014 at 8:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>>
>>> ChangeLog for testsuite:
>>>
>>> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>>>
>>>         PR target/8340
>>>         PR middle-end/47602
>>>         PR rtl-optimization/55458
>>>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>>>         * gcc.target/i386/pr55458.c: Likewise.
>>>         * gcc.target/i386/pr47602.c: New.
>>>         * gcc.target/i386/pr23098.c: Move to XFAIL.
>>
>> Reversed patch was attached. Please repost.

OK.

Thanks,
Uros.

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[H.J. Lu]

On Mon, Oct 13, 2014 at 9:32 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> Reattached.
>
> On Mon, Oct 13, 2014 at 8:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>>
>>> ChangeLog for testsuite:
>>>
>>> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>>>
>>>         PR target/8340
>>>         PR middle-end/47602
>>>         PR rtl-optimization/55458
>>>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>>>         * gcc.target/i386/pr55458.c: Likewise.
>>>         * gcc.target/i386/pr47602.c: New.
>>>         * gcc.target/i386/pr23098.c: Move to XFAIL.
>>
>> Reversed patch was attached. Please repost.
>>
>> Uros.

This caused a regression:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63527

-- 
H.J.

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jakub Jelinek]

On Fri, Oct 10, 2014 at 10:03:38AM -0600, Jeff Law wrote:
> Can you add a PR markers to your changelog
> 
> 	PR target/8340
> 	PR middle-end/47602
> 	PR rtl-optimization/55458
> 
> Actually I think there is an additional test in 47602.  Can you please add
> it to the suite?  You'll also want to change the state of 47602 to
> RESOLVED/FIXED.

Unfortunately this broke bootstrap on x86_64/i686-linux,
see http://gcc.gnu.org/PR63534
- pretty much everything with -m32 -fsplit-stack -fpic ICEs, -m32 -fpic -p
results in wrong-code, and I see significant code quality regressions even
on simple testcases.

For the first two, I think (and said it before already) that the current
model of emitting set_got from a target hook during RA can't work, as there
can be calls in the prologue, and the prologue is inserted before the
set_got in that case.  I really think the RA should in that case just tell
the backend whether and in which register it wants to have the PIC register
loaded upon start of the function, and it should be emit prologue pass
that should arrange for that.

As for the code quality, either some RA improvements are needed, or
postreload must be able to fix it up, or hardreg propagation (though,
cprop_hardreg is forward propagation rather than backwards, right?).
Better before prologue is emitted though, because that will save/restore
the badly chosen hard reg too.

	Jakub

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[H.J. Lu]

On Mon, Oct 13, 2014 at 11:49 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Oct 13, 2014 at 9:32 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>> Reattached.
>>
>> On Mon, Oct 13, 2014 at 8:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>>>
>>>> ChangeLog for testsuite:
>>>>
>>>> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>>>>
>>>>         PR target/8340
>>>>         PR middle-end/47602
>>>>         PR rtl-optimization/55458
>>>>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>>>>         * gcc.target/i386/pr55458.c: Likewise.
>>>>         * gcc.target/i386/pr47602.c: New.
>>>>         * gcc.target/i386/pr23098.c: Move to XFAIL.
>>>
>>> Reversed patch was attached. Please repost.
>>>
>>> Uros.
>
> This caused a regression:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63527

Another bootstrap failure:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63536

-- 
H.J.

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jeff Law]

On 10/14/14 07:00, Jakub Jelinek wrote:
>
> For the first two, I think (and said it before already) that the current
> model of emitting set_got from a target hook during RA can't work, as there
> can be calls in the prologue, and the prologue is inserted before the
> set_got in that case.  I really think the RA should in that case just tell
> the backend whether and in which register it wants to have the PIC register
> loaded upon start of the function, and it should be emit prologue pass
> that should arrange for that.
That works for me -- I've been encouraging Intel to push emitting the 
PIC setup further and further back in the RTL pipeline.  Their early 
patches had it very early in the RTL pipeline and naturally there was 
fallout/bleedout in various places in the optimizers.

I don't see much value in emitting the PIC setup prior to allocation, 
all I see is problems.


>
> As for the code quality, either some RA improvements are needed, or
> postreload must be able to fix it up, or hardreg propagation (though,
> cprop_hardreg is forward propagation rather than backwards, right?).
> Better before prologue is emitted though, because that will save/restore
> the badly chosen hard reg too.
RA improvements are the way to go -- however, my understanding is that 
overall the code is better now than it was before Intel's changes, so I 
don't consider the performance side as a blocker for this code.

The biggest performance issue identified so far is rematerialization. 
The initial patch Intel sent to me was totally unacceptable as it just 
hacked off optimizers rather than digging into the guts of why IRA/LRA 
was unable to sanely rematerialize the PIC register value.

jeff

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[H.J. Lu]

On Tue, Oct 14, 2014 at 9:43 AM, Jeff Law <law@redhat.com> wrote:
>
> RA improvements are the way to go -- however, my understanding is that
> overall the code is better now than it was before Intel's changes, so I
> don't consider the performance side as a blocker for this code.
>

The new approach improves PIC code quality in functions where there
no frequent GOT access and extra register helps.

For ld.so and libc.so from glibc build, we use 2 registers to access GOT
instead of one register which may lead to lower performance in shared
libraries.


-- 
H.J.

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Rainer Orth]

Evgeny Stupachenko <evstupac@gmail.com> writes:

> Reattached.
>
> On Mon, Oct 13, 2014 at 8:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com>
>> wrote:
>>
>>> ChangeLog for testsuite:
>>>
>>> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>>>
>>>         PR target/8340
>>>         PR middle-end/47602
>>>         PR rtl-optimization/55458
>>>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>>>         * gcc.target/i386/pr55458.c: Likewise.
>>>         * gcc.target/i386/pr47602.c: New.
>>>         * gcc.target/i386/pr23098.c: Move to XFAIL.

The unconditional XFAIL is wrong: the test now XPASSes on
i386-pc-solaris2.11 and x86_64-unknown-linux-gnu, i686-unknown-linux-gnu
for 32-bit.  Please fix.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

The test passes now. So let's remove xfail.

2014-10-29  Evgeny Stupachenko  <evstupac@gmail.com>

gcc/testsuite
        * gcc.target/i386/pr23098.c: Remove xfail.

diff --git a/gcc/testsuite/gcc.target/i386/pr23098.c
b/gcc/testsuite/gcc.target/i386/pr23098.c
index 7f118dc..7f118bb 100644
--- a/gcc/testsuite/gcc.target/i386/pr23098.c
+++ b/gcc/testsuite/gcc.target/i386/pr23098.c
@@ -1,7 +1,7 @@
 /* PR rtl-optimization/23098 */
 /* { dg-do compile } */
 /* { dg-options "-O2 -fPIC" } */
-/* { dg-final { scan-assembler-not "\.LC\[0-9\]" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-not "\.LC\[0-9\]" } } */
 /* { dg-require-effective-target ia32 } */
 /* { dg-require-effective-target fpic } */

On Thu, Oct 23, 2014 at 4:19 PM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> Evgeny Stupachenko <evstupac@gmail.com> writes:
>
>> Reattached.
>>
>> On Mon, Oct 13, 2014 at 8:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Mon, Oct 13, 2014 at 4:53 PM, Evgeny Stupachenko <evstupac@gmail.com>
>>> wrote:
>>>
>>>> ChangeLog for testsuite:
>>>>
>>>> 2014-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
>>>>
>>>>         PR target/8340
>>>>         PR middle-end/47602
>>>>         PR rtl-optimization/55458
>>>>         * gcc.target/i386/pic-1.c: Remove dg-error as test should pass now.
>>>>         * gcc.target/i386/pr55458.c: Likewise.
>>>>         * gcc.target/i386/pr47602.c: New.
>>>>         * gcc.target/i386/pr23098.c: Move to XFAIL.
>
> The unconditional XFAIL is wrong: the test now XPASSes on
> i386-pc-solaris2.11 and x86_64-unknown-linux-gnu, i686-unknown-linux-gnu
> for 32-bit.  Please fix.
>
>         Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH 1/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Uros Bizjak]

On Wed, Oct 29, 2014 at 1:28 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> The test passes now. So let's remove xfail.
>
> 2014-10-29  Evgeny Stupachenko  <evstupac@gmail.com>
>
> gcc/testsuite
>         * gcc.target/i386/pr23098.c: Remove xfail.

OK.

Thanks,
Uros.