[Bug bootstrap/44470] New: [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[Bug bootstrap/44470] New: [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

Revision 160417 failed to compile libgcc:

[hjl@gnu-32 rrs]$ cat testcase.i
typedef int SItype __attribute__ ((mode (SI)));
typedef unsigned int USItype __attribute__ ((mode (SI)));
typedef int DItype __attribute__ ((mode (DI)));
  struct DWstruct {SItype low, high;};
typedef union
{
  struct DWstruct s;
  DItype ll;
} DWunion;
DItype
__negdi2 (DItype u)
{
  const DWunion uu = {.ll = u};
  const DWunion w = { {.low = -uu.s.low,
         .high = -uu.s.high - ((USItype) -uu.s.low > 0) } };
  return w.ll;
}
[hjl@gnu-32 rrs]$ /export/gnu/import/rrs/160417/usr/bin/gcc  -m32 -march=atom
-O2 -fPIC -S testcase.i   
testcase.i: In function ‘__negdi2’:
testcase.i:17:1: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
[hjl@gnu-32 rrs]$


-- 
           Summary: [4.6 Regression] Failed to bootstrap with - -with-
                    arch=atom
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: bootstrap
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl dot tools at gmail dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #1 from hjl dot tools at gmail dot com  2010-06-08 22:00 -------
It may be broken by revision 160394:

http://gcc.gnu.org/ml/gcc-cvs/2010-06/msg00307.html


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |uros
            Summary|[4.6 Regression] Failed to  |[4.6 Regression] Failed to
                   |bootstrap with - -with-     |bootstrap with - -with-
                   |arch=atom                   |arch=atom


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #2 from hjl dot tools at gmail dot com  2010-06-09 00:52 -------
(In reply to comment #1)
> It may be broken by revision 160394:
> 
> http://gcc.gnu.org/ml/gcc-cvs/2010-06/msg00307.html
> 

This change moved

(insn:TI 11 41 12 pr44470.i:15 (parallel [
            (set (reg:SI 1 dx [orig:61 w$s$high ] [61])
                (plus:SI (reg:SI 2 cx [69])
                    (reg:SI 1 dx [orig:75 uu+4 ] [75])))
            (clobber (reg:CC 17 flags))
        ]) 251 {*addsi_1} (expr_list:REG_DEAD (reg:SI 2 cx [69])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

(insn 12 11 23 pr44470.i:15 (parallel [
            (set (reg:SI 1 dx [orig:61 w$s$high ] [61])
                (neg:SI (reg:SI 1 dx [orig:61 w$s$high ] [61])))
            (clobber (reg:CC 17 flags))
        ]) 442 {*negsi2_1} (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

into epilogue.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #3 from hjl dot tools at gmail dot com  2010-06-09 00:59 -------
The old scheduler:

;;   ======================================================
;;   -- basic block 2 from 37 to 42 -- after reload
;;   ======================================================

;;        0-->    37 [--sp]=bp                         :atom-dual-2c
;;        1-->    44 {cx=0;clobber flags;}             :atom-simple-either
;;        2-->    38 bp=sp                             :atom-simple-either
;;        6-->    26 ax=[bp+0x8]                       :atom-simple-either
;;        6-->    27 dx=[bp+0xc]                       :atom-simple-either
;;        7-->    45 flags=cmp(ax,0)                   :atom-simple-either
;;        8-->    46 strict_low_part=flags!=0          :atom-simple-either
;;        8-->    13 {ax=-ax;clobber flags;}           :atom-simple-either
;;        9-->    11 {dx=cx+dx;clobber flags;}         :atom-simple-either
;;       10-->    12 {dx=-dx;clobber flags;}           :atom-simple-either
;;       10-->    23 use ax                            :nothing
;;       11-->    41 {bp=[sp];sp=sp+0x4;}              :atom-dual-2c
;;       12-->    42 return                            :atom-dual-1c
;;      Ready list (final):  
;;   total time = 12
;;   new head = 37
;;   new tail = 42

The new one:
;;   ======================================================
;;   -- basic block 2 from 37 to 42 -- after reload
;;   ======================================================

;;        0-->    37 [--sp]=bp                         :atom-dual-2c
;;        1-->    44 {cx=0;clobber flags;}             :atom-simple-either
;;        2-->    38 bp=sp                             :atom-simple-either
;;        6-->    26 ax=[bp+0x8]                       :atom-simple-either
;;        6-->    27 dx=[bp+0xc]                       :atom-simple-either
;;        7-->    45 flags=cmp(ax,0)                   :atom-simple-either
;;        8-->    46 strict_low_part=flags!=0          :atom-simple-either
;;        8-->    13 {ax=-ax;clobber flags;}           :atom-simple-either
;;        9-->    41 {bp=[sp];sp=sp+0x4;}              :atom-dual-2c
;;       12-->    11 {dx=cx+dx;clobber flags;}         :atom-simple-either
;;       12-->    12 {dx=-dx;clobber flags;}           :atom-simple-either
;;       13-->    23 use ax                            :nothing
;;       13-->    42 return                            :atom-dual-1c
;;      Ready list (final):  
;;   total time = 13
;;   new head = 37
;;   new tail = 42

It is slower.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #4 from ubizjak at gmail dot com  2010-06-09 06:27 -------
(In reply to comment #1)
> It may be broken by revision 160394:
> 
> http://gcc.gnu.org/ml/gcc-cvs/2010-06/msg00307.html

The add->lea transformation doesn't even trigger in this testcase... You still
have normal add instruction with a flags clobber up to and including sched2
pass.

Later, compilation breaks in machine reorg pass with:

Program received signal SIGSEGV, Segmentation fault.
distance_non_agu_define (code=<value optimized out>, insn=0x7ffff18c2750, 
    operands=0x1298aa0) at ../../gcc-svn/trunk/gcc/config/i386/i386.c:13826
13826     if (insn != BB_HEAD (bb))

(gdb) l
13821     basic_block bb = BLOCK_FOR_INSN (insn);
13822     int distance = 0;
13823     df_ref *def_rec;
13824     enum attr_type insn_type;
13825   
13826     if (insn != BB_HEAD (bb))
13827       {
13828         rtx prev = PREV_INSN (insn);
13829         while (prev && distance < LEA_SEARCH_THRESHOLD)
13830           {

It looks to me that bb is NULL, which isn't a good sign anyway.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #5 from ubizjak at gmail dot com  2010-06-09 07:46 -------
Looking into it.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ubizjak at gmail dot com
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-06-09 07:46:05
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #6 from ubizjak at gmail dot com  2010-06-09 12:15 -------
Following patch is also needed to fix conditional splitting (it does not fix
original uncovered problem where BLOCK_FOR_INSN returns null):

Index: i386.md
===================================================================
--- i386.md     (revision 160445)
+++ i386.md     (working copy)
@@ -6087,8 +6087,15 @@
   switch (get_attr_type (insn))
     {
     case TYPE_LEA:
-      return "#";
+      if (reload_completed && ix86_lea_for_add_ok (PLUS, insn, operands))
+        return "#";

+      gcc_assert (rtx_equal_p (operands[0], operands[2]));
+      if (x86_maybe_negate_const_int (&operands[1], <MODE>mode))
+        return "sub{<imodesuffix>}\t{%1, %0|%0, %1}";
+
+      return "add{<imodesuffix>}\t{%1, %0|%0, %1}";
+
     case TYPE_INCDEC:
       gcc_assert (rtx_equal_p (operands[0], operands[1]));
       if (operands[2] == const1_rtx)
@@ -6138,8 +6145,14 @@
   switch (get_attr_type (insn))
     {
     case TYPE_LEA:
-      return "#";
+      if (reload_completed && ix86_lea_for_add_ok (PLUS, insn, operands))
+        return "#";

+      if (x86_maybe_negate_const_int (&operands[1], SImode))
+        return "sub{l}\t{%1, %k0|%k0, %1}";
+
+      return "add{l}\t{%1, %k0|%k0, %1}";
+
     case TYPE_INCDEC:
       if (operands[2] == const1_rtx)
         return "inc{l}\t%k0";
@@ -6222,8 +6235,15 @@
   switch (get_attr_type (insn))
     {
     case TYPE_LEA:
-      return "#";
+      if (reload_completed && ix86_lea_for_add_ok (PLUS, insn, operands))
+        return "#";

+      gcc_assert (rtx_equal_p (operands[0], operands[2]));
+      if (x86_maybe_negate_const_int (&operands[1], HImode))
+        return "sub{w}\t{%1, %0|%0, %1}";
+
+      return "add{w}\t{%1, %0|%0, %1}";
+
     case TYPE_INCDEC:
       gcc_assert (rtx_equal_p (operands[0], operands[1]));
       if (operands[2] == const1_rtx)
@@ -6313,8 +6333,22 @@
   switch (get_attr_type (insn))
     {
     case TYPE_LEA:
-      return "#";
+      if (reload_completed && ix86_lea_for_add_ok (PLUS, insn, operands))
+        return "#";

+      gcc_assert (rtx_equal_p (operands[0], operands[2]));
+      if (x86_maybe_negate_const_int (&operands[1], QImode))
+       {
+         if (widen)
+           return "sub{l}\t{%1, %k0|%k0, %1}";
+         else
+           return "sub{b}\t{%1, %0|%0, %1}";
+       }
+      if (widen)
+        return "add{l}\t{%k1, %k0|%k0, %k1}";
+      else
+        return "add{b}\t{%1, %0|%0, %1}";
+
     case TYPE_INCDEC:
       gcc_assert (rtx_equal_p (operands[0], operands[1]));
       if (operands[2] == const1_rtx)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #7 from hjl dot tools at gmail dot com  2010-06-09 14:13 -------
(In reply to comment #4)
> (In reply to comment #1)
> > It may be broken by revision 160394:
> > 
> > http://gcc.gnu.org/ml/gcc-cvs/2010-06/msg00307.html
> 
> The add->lea transformation doesn't even trigger in this testcase... You still
> have normal add instruction with a flags clobber up to and including sched2
> pass.
> 
> Later, compilation breaks in machine reorg pass with:
> 
> Program received signal SIGSEGV, Segmentation fault.
> distance_non_agu_define (code=<value optimized out>, insn=0x7ffff18c2750, 
>     operands=0x1298aa0) at ../../gcc-svn/trunk/gcc/config/i386/i386.c:13826
> 13826     if (insn != BB_HEAD (bb))
> 
> (gdb) l
> 13821     basic_block bb = BLOCK_FOR_INSN (insn);
> 13822     int distance = 0;
> 13823     df_ref *def_rec;
> 13824     enum attr_type insn_type;
> 13825   
> 13826     if (insn != BB_HEAD (bb))
> 13827       {
> 13828         rtx prev = PREV_INSN (insn);
> 13829         while (prev && distance < LEA_SEARCH_THRESHOLD)
> 13830           {
> 
> It looks to me that bb is NULL, which isn't a good sign anyway.

ix86_lea_for_add_ok shouldn't be call during final scan when
we have already looked all add patterns and converted them
to lea if necessary.


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.0                       |---


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #8 from hjl dot tools at gmail dot com  2010-06-09 14:16 -------
Whatever we do, we need to preserve Atom add->lea optimization.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #9 from hjl dot tools at gmail dot com  2010-06-09 20:30 -------
(In reply to comment #6)
> Following patch is also needed to fix conditional splitting (it does not fix
> original uncovered problem where BLOCK_FOR_INSN returns null):
> 
>

I am not sure this is correct. The code prior to revision 160394 was
written in such a way to support X86_TUNE_OPT_AGU.  We may have missed
some cases. But it is mostly correct. I'd like to know what specific
cases revision 160394 tried to fix. Thanks.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
   Target Milestone|---                         |4.6.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #10 from ubizjak at gmail dot com  2010-06-10 06:18 -------
(In reply to comment #9)

> > Following patch is also needed to fix conditional splitting (it does not fix
> > original uncovered problem where BLOCK_FOR_INSN returns null):
> 
> I am not sure this is correct. The code prior to revision 160394 was
> written in such a way to support X86_TUNE_OPT_AGU.  We may have missed
> some cases. But it is mostly correct. I'd like to know what specific
> cases revision 160394 tried to fix. Thanks.

The patch fixes general problem, where we simply output lea from add{si,di}
instruction. This is not correct, since lea does not clobber flags, so we
should split add RTX to lea RTX to model this fact. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #11 from hjl dot tools at gmail dot com  2010-06-10 14:30 -------
(In reply to comment #10)
> (In reply to comment #9)
> 
> > > Following patch is also needed to fix conditional splitting (it does not fix
> > > original uncovered problem where BLOCK_FOR_INSN returns null):
> > 
> > I am not sure this is correct. The code prior to revision 160394 was
> > written in such a way to support X86_TUNE_OPT_AGU.  We may have missed
> > some cases. But it is mostly correct. I'd like to know what specific
> > cases revision 160394 tried to fix. Thanks.
> 
> The patch fixes general problem, where we simply output lea from add{si,di}
> instruction. This is not correct, since lea does not clobber flags, so we
> should split add RTX to lea RTX to model this fact. 
> 

ADD is always faster than LEA for adding a register. However
there is a special case on Atom where ADD should be avoided.
It is true that LEA doesn't touch flags and we used it instead
of ADD which touches flags. It is an optimization specific for
Atom. You can think it as a special ADD for Atom and pretend
it clobbers flags. We shouldn't model it as a real LEA since
it is used to implement a faster ADD in this special case.

Uros, I'd like to revert revision 160394 to fix bootstrap on
Atom. I will work with you to resolve issues you discovered
without affecting Atom performance.  Thanks.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #12 from ubizjak at gmail dot com  2010-06-10 14:57 -------
(In reply to comment #11)

> > > I am not sure this is correct. The code prior to revision 160394 was
> > > written in such a way to support X86_TUNE_OPT_AGU.  We may have missed
> > > some cases. But it is mostly correct. I'd like to know what specific
> > > cases revision 160394 tried to fix. Thanks.
> > 
> > The patch fixes general problem, where we simply output lea from add{si,di}
> > instruction. This is not correct, since lea does not clobber flags, so we
> > should split add RTX to lea RTX to model this fact. 
> > 
> 
> ADD is always faster than LEA for adding a register. However
> there is a special case on Atom where ADD should be avoided.
> It is true that LEA doesn't touch flags and we used it instead
> of ADD which touches flags. It is an optimization specific for
> Atom. You can think it as a special ADD for Atom and pretend
> it clobbers flags. We shouldn't model it as a real LEA since
> it is used to implement a faster ADD in this special case.
> 
> Uros, I'd like to revert revision 160394 to fix bootstrap on
> Atom. I will work with you to resolve issues you discovered
> without affecting Atom performance.  Thanks.

OK, please go ahead. The correct fix is not as simple as I have thought.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #13 from ubizjak at gmail dot com  2010-06-10 14:59 -------
(In reply to comment #12)
> (In reply to comment #11)
> 
> > > > I am not sure this is correct. The code prior to revision 160394 was
> > > > written in such a way to support X86_TUNE_OPT_AGU.  We may have missed
> > > > some cases. But it is mostly correct. I'd like to know what specific
> > > > cases revision 160394 tried to fix. Thanks.
> > > 
> > > The patch fixes general problem, where we simply output lea from add{si,di}
> > > instruction. This is not correct, since lea does not clobber flags, so we
> > > should split add RTX to lea RTX to model this fact. 
> > > 
> > 
> > ADD is always faster than LEA for adding a register. However
> > there is a special case on Atom where ADD should be avoided.
> > It is true that LEA doesn't touch flags and we used it instead
> > of ADD which touches flags. It is an optimization specific for
> > Atom. You can think it as a special ADD for Atom and pretend
> > it clobbers flags. We shouldn't model it as a real LEA since
> > it is used to implement a faster ADD in this special case.
> > 
> > Uros, I'd like to revert revision 160394 to fix bootstrap on
> > Atom. I will work with you to resolve issues you discovered
> > without affecting Atom performance.  Thanks.
> 
> OK, please go ahead. The correct fix is not as simple as I have thought.

Please revert only this part:

        * config/i386/i386.md (*add<mode>_1): Remove alternative 2.
        <TYPE_LEA>: Split instruction.
        <default>: Remove alternative 2 handling.
        (*addsi_1_zext) <TYPE_LEA>: Split instruction.
        (add lea splitter): Generate SImode lea for mode sizes <= SImode.
        (add_zext lea splitter): Use ix86_lea_for_add_ok in insn predicate.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #14 from ubizjak at gmail dot com  2010-06-10 15:12 -------
(In reply to comment #11)

> ADD is always faster than LEA for adding a register. However
> there is a special case on Atom where ADD should be avoided.
> It is true that LEA doesn't touch flags and we used it instead
> of ADD which touches flags. It is an optimization specific for
> Atom. You can think it as a special ADD for Atom and pretend
> it clobbers flags. We shouldn't model it as a real LEA since
> it is used to implement a faster ADD in this special case.

LEA can also implement true three-operand add (rX = rY + rZ), so it can save a
move. Expansion of three operands is effectively disabled due to usage of
ix86_binary_operand_ok in PLUS patterns, so split to LEA certainly has some
benefit.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #15 from hjl dot tools at gmail dot com  2010-06-10 15:43 -------
(In reply to comment #14)
> (In reply to comment #11)
> 
> > ADD is always faster than LEA for adding a register. However
> > there is a special case on Atom where ADD should be avoided.
> > It is true that LEA doesn't touch flags and we used it instead
> > of ADD which touches flags. It is an optimization specific for
> > Atom. You can think it as a special ADD for Atom and pretend
> > it clobbers flags. We shouldn't model it as a real LEA since
> > it is used to implement a faster ADD in this special case.
> 
> LEA can also implement true three-operand add (rX = rY + rZ), so it can save a
> move. Expansion of three operands is effectively disabled due to usage of
> ix86_binary_operand_ok in PLUS patterns, so split to LEA certainly has some
> benefit.
> 

We only want to use LEAD instead ADD for rX += rY
under certain conditions. As I said, ADD is faster
than LEA in most cases.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl at gcc dot gnu dot org]

------- Comment #16 from hjl at gcc dot gnu dot org  2010-06-10 16:01 -------
Subject: Bug 44470

Author: hjl
Date: Thu Jun 10 16:00:31 2010
New Revision: 160557

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=160557
Log:
Revert the part of r160394 to fix bootstap with --with-arch=atom.

2010-06-10  H.J. Lu  <hongjiu.lu@intel.com>

        PR boostrap/44470
        * config/i386/i386.md (*add<mode>_1): Revert revision 160394.
        (*addsi_1_zext) <TYPE_LEA>: Likewise.
        (add lea splitter): Likewise.
        (add_zext lea splitter): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.md


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #17 from hjl dot tools at gmail dot com  2010-06-10 19:16 -------
Fixed.


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[bernds at gcc dot gnu dot org]

------- Comment #18 from bernds at gcc dot gnu dot org  2010-08-16 23:49 -------
I'm seeing some strange situations where this code is unnecessarily producing
lea insns even when not tuning for Atom.

This code looks very strange.  I don't understand why we aren't splitting to a
lea pattern anymore if we want to produce a lea insn.

Also, isn't the test for TARGET_OPT_AGU reversed?


-- 

bernds at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[bernds at gcc dot gnu dot org]

------- Comment #19 from bernds at gcc dot gnu dot org  2010-08-16 23:51 -------
Created an attachment (id=21497)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21497&action=view)
A patch which should produce more add insns

In other words, don't we at least need this patch to avoid generating
unnecessary leas?

But still, alternative 2 of addsi_1 looks extremely ugly, and I think the code
before revision 160557 made more sense.  What exactly was wrong with it?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #20 from hjl dot tools at gmail dot com  2010-08-17 00:10 -------
(In reply to comment #18)
> I'm seeing some strange situations where this code is unnecessarily producing
> lea insns even when not tuning for Atom.
> 
> This code looks very strange.  I don't understand why we aren't splitting to a
> lea pattern anymore if we want to produce a lea insn.
> 
> Also, isn't the test for TARGET_OPT_AGU reversed?
> 

Do you have a testcase?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #21 from hjl dot tools at gmail dot com  2010-08-17 00:11 -------
(In reply to comment #19)
> Created an attachment (id=21497)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21497&action=view) [edit]
> A patch which should produce more add insns
> 
> In other words, don't we at least need this patch to avoid generating
> unnecessary leas?
> 
> But still, alternative 2 of addsi_1 looks extremely ugly, and I think the code
> before revision 160557 made more sense.  What exactly was wrong with it?
> 

Do you have a testcase to show how it changes the generated assembly
codes?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[bernds at gcc dot gnu dot org]

------- Comment #22 from bernds at gcc dot gnu dot org  2010-08-17 00:16 -------
I was looking at Spec2k/254.gap/integer.s which has many examples, all of the
form.

-       leal    (%ecx,%eax), %eax
+       addl    %ecx, %eax


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #23 from hjl dot tools at gmail dot com  2010-08-17 03:46 -------
Created an attachment (id=21499)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21499&action=view)
A different patch

We added the 2rd alternative to "*add<mode>_1" for Atom
so that we always use add instead lea. It looks like a win
for everyone. We can remove AGU check.

As for ix86_lea_for_add_ok, it can be simpler.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[bernds at gcc dot gnu dot org]

------- Comment #24 from bernds at gcc dot gnu dot org  2010-08-17 09:47 -------
I think that's beginning to look reasonable.  So the problem was that without
alternative 2, such an add would match alternative 3 instead and be split?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[hjl dot tools at gmail dot com]

------- Comment #25 from hjl dot tools at gmail dot com  2010-08-17 14:47 -------
(In reply to comment #24)
> I think that's beginning to look reasonable.  So the problem was that without
> alternative 2, such an add would match alternative 3 instead and be split?
> 

Yes.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470

[Bug bootstrap/44470] [4.6 Regression] Failed to bootstrap with - -with-arch=atom

[ubizjak at gmail dot com]

------- Comment #26 from ubizjak at gmail dot com  2010-08-18 20:13 -------
Splitting to LEA was fixed in r163351 [1] with patch at [2].

[1] http://gcc.gnu.org/ml/gcc-cvs/2010-08/msg00562.html
[2] http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01394.html


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44470