diff mbox

[PR,target/65103,1/3] Fix cost of PIC register in ix86_address_cost

Message ID 20150312095055.GJ27860@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich March 12, 2015, 9:50 a.m. UTC 
On 10 Mar 19:08, Uros Bizjak wrote:
> Hello!
> 
> > > > Test         O2 ref     patched    Ofast + LTO ref   patched
> > > > 164.gzip        12      0 (-100%)            39      0 (-100%)
> > > > 175.vpr         0       0 (-0%)              4       0 (-100%)
> > > > 176.gcc         141     6 (-96%)             294     10 (-97%)
> > > > 181.mcf         4       0 (-100%)            4       2 (-50%)
> 
> Do you also have executable sizes at hand?

Summary size change for SPEC2000 on -O2 is -0,11%.

> 
> > 2015-03-10  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> > PR target/65103
> > * config/i386/i386.c (ix86_address_cost): Fix cost of a PIC
> > register.
> >
> > gcc/testsuite/
> >
> > 2015-03-10  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> > PR target/65103
> > * gcc.target/i386/pr65103-1.c: New.
> 
> LGTM, just a nit below.
> 
> Otherwise, OK for mainline as a bugfix (but please wait for a day if
> there are any objections from release managers).
> 
> +  /* Attempt to minimize number of registers in the address.
> 
> This is now a displaced comment. Please integrate it in the main comment.
> 
> Thanks,
> Uros.

Here is a final version.

Thanks,
Ilya
--

Comments

Uros Bizjak March 12, 2015, 9:59 a.m. UTC | #1 
On Thu, Mar 12, 2015 at 10:50 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:

>> > > > Test         O2 ref     patched    Ofast + LTO ref   patched
>> > > > 164.gzip        12      0 (-100%)            39      0 (-100%)
>> > > > 175.vpr         0       0 (-0%)              4       0 (-100%)
>> > > > 176.gcc         141     6 (-96%)             294     10 (-97%)
>> > > > 181.mcf         4       0 (-100%)            4       2 (-50%)
>>
>> Do you also have executable sizes at hand?
>
> Summary size change for SPEC2000 on -O2 is -0,11%.

Nice!

>> > 2015-03-10  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> > PR target/65103
>> > * config/i386/i386.c (ix86_address_cost): Fix cost of a PIC
>> > register.
>> >
>> > gcc/testsuite/
>> >
>> > 2015-03-10  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> > PR target/65103
>> > * gcc.target/i386/pr65103-1.c: New.
>>
>> LGTM, just a nit below.
>>
>> Otherwise, OK for mainline as a bugfix (but please wait for a day if
>> there are any objections from release managers).
>>
>> +  /* Attempt to minimize number of registers in the address.
>>
>> This is now a displaced comment. Please integrate it in the main comment.
>>
>> Thanks,
>> Uros.
>
> Here is a final version.

OK for mainline.

Thanks,
Uros.
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ab8f03a..47deda7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12931,30 +12931,26 @@  ix86_address_cost (rtx x, machine_mode, addr_space_t, bool)
   if (parts.index && GET_CODE (parts.index) == SUBREG)
     parts.index = SUBREG_REG (parts.index);
 
-  /* Attempt to minimize number of registers in the address.  */
-  if ((parts.base
-       && (!REG_P (parts.base) || REGNO (parts.base) >= FIRST_PSEUDO_REGISTER))
-      || (parts.index
-	  && (!REG_P (parts.index)
-	      || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER)))
-    cost++;
-
-  /* When address base or index is "pic_offset_table_rtx" we don't increase
-     address cost.  When a memopt with "pic_offset_table_rtx" is not invariant
-     itself it most likely means that base or index is not invariant.
-     Therefore only "pic_offset_table_rtx" could be hoisted out, which is not
-     profitable for x86.  */
+  /* Attempt to minimize number of registers in the address by increasing
+     address cost for each used register.  We don't increase address cost
+     for "pic_offset_table_rtx".  When a memopt with "pic_offset_table_rtx"
+     is not invariant itself it most likely means that base or index is not
+     invariant.  Therefore only "pic_offset_table_rtx" could be hoisted out,
+     which is not profitable for x86.  */
   if (parts.base
-      && (current_pass->type == GIMPLE_PASS
-	  || (!pic_offset_table_rtx
-	      || REGNO (pic_offset_table_rtx) != REGNO(parts.base)))
       && (!REG_P (parts.base) || REGNO (parts.base) >= FIRST_PSEUDO_REGISTER)
-      && parts.index
       && (current_pass->type == GIMPLE_PASS
-	  || (!pic_offset_table_rtx
-	      || REGNO (pic_offset_table_rtx) != REGNO(parts.index)))
+	  || !pic_offset_table_rtx
+	  || !REG_P (parts.base)
+	  || REGNO (pic_offset_table_rtx) != REGNO (parts.base)))
+    cost++;
+
+  if (parts.index
       && (!REG_P (parts.index) || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER)
-      && parts.base != parts.index)
+      && (current_pass->type == GIMPLE_PASS
+	  || !pic_offset_table_rtx
+	  || !REG_P (parts.index)
+	  || REGNO (pic_offset_table_rtx) != REGNO (parts.index)))
     cost++;
 
   /* AMD-K6 don't like addresses with ModR/M set to 00_xxx_100b,
diff --git a/gcc/testsuite/gcc.target/i386/pr65103-1.c b/gcc/testsuite/gcc.target/i386/pr65103-1.c
new file mode 100644
index 0000000..4e3a7a3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65103-1.c
@@ -0,0 +1,19 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-require-effective-target pie } */
+/* { dg-options "-O2 -fPIE" } */
+/* { dg-final { scan-assembler-not "GOTOFF," } } */
+
+typedef struct S
+{
+  int a;
+  int sum;
+  int delta;
+} S;
+
+S gs;
+int global_opt (int max)
+{
+  while (gs.sum < max)
+    gs.sum += gs.delta;
+  return gs.a;
+}