[Bug c/16028] New: Bad code gen for vector code with -O3 (caused by GRA/reload)

[Bug c/16028] New: Bad code gen for vector code with -O3 (caused by GRA/reload)

[fjahanian at apple dot com]

Following test case when compiled with -O3 shows a bug in reload, used by GRA. 
Test case:

typedef short __attribute__ ((vector_size (16))) vecint;

vecint i = { 150, 100, 150, 200, 0, 0, 0, 0 };vecint j = { 10, 13, 20, 30, 1, 1, 1, 1 };
vecint k;

union {
  vecint v;
  short i[8];
} res;
void
verify (int a1, int b1)
{
  if (a1 != b1)
    abort ();}

int
main ()
{  k = i + j;
  res.v = k;

  verify (res.i[0], 160);

  k = i & j; /* bad code gen for this statement causes the abort. */
  res.v = k;
  verify (res.i[0], 2);

  k = i * j;
  res.v = k;
  verify (res.i[0], 1500);

  exit (0);
}

Assembly code for powerpc darwin shows the bug:

        lhz r16,78(r1)  ! Only half-word is loaded
        ...
        lhz r20,94(r1)  ! Only half-word is loaded
        ...
        and r28,r16,r20 ! expects full 32bit values in r16 and r20 here
        and r29,r17,r21
        ...

Followin is the relevant RTL generated after LRA and before GRA.

From: b.c.25.lreg 

(insn:HI 11 18 29 0 (set (subreg:SI (reg:V8HI 118 [ i.0 ]) 0)
        (mem:SI (lo_sum:SI (reg/f:SI 125)
                (symbol_ref:SI ("!D__i") [flags 0x2] <var_decl 0x41700480 i>)) [0 i+0 S4 A128])) 303 
{movsi_low} (insn_list 8 (insn_list 18 (nil)))
    (nil))
...

(insn:HI 13 301 24 0 (set (subreg:SI (reg:V8HI 118 [ i.0 ]) 4)
        (mem:SI (plus:SI (reg/f:SI 124)
                (const_int 4 [0x4])) [0 i+4 S4 A32])) 309 {*movsi_internal1} (insn_list 9 (insn_list 11 (nil)))
    (nil))
...

(insn:HI 30 307 33 0 (set (reg:TI 136 [ i.0 ])
        (subreg:TI (reg:V8HI 118 [ i.0 ]) 0)) 331 {*movti_string} (insn_list 17 (nil))
    (nil))
...

(insn:HI 40 38 41 0 (set (reg:HI 145)
        (subreg:HI (reg:V8HI 118 [ i.0 ]) 2)) 311 {*movhi_internal} (insn_list 17 (nil))
    (nil))
...

(insn:HI 54 52 55 0 (set (reg:HI 157)
        (subreg:HI (reg:V8HI 118 [ i.0 ]) 6)) 311 {*movhi_internal} (insn_list 17 (nil))
    (nil))
...

(insn:HI 68 66 69 0 (set (reg:HI 169)
        (subreg:HI (reg:V8HI 118 [ i.0 ]) 10)) 311 {*movhi_internal} (insn_list 17 (nil))
    (nil))

...

(insn:HI 82 80 83 0 (set (reg:HI 181 [ i.0 ])
        (subreg:HI (reg:V8HI 118 [ i.0 ]) 14)) 311 {*movhi_internal} (insn_list 17 (nil))
    (nil))

...

(insn:HI 121 152 122 2 (parallel [
            (set (subreg:SI (reg:V8HI 121 [ T.6 ]) 0)
                (and:SI (subreg:SI (reg:V8HI 118 [ i.0 ]) 0)
                    (subreg:SI (reg:V8HI 119 [ j.1 ]) 0)))
            (clobber (scratch:CC))
        ]) 81 {andsi3} (nil)
    (expr_list:REG_UNUSED (scratch:CC)
        (nil)))

===========================================================

During reload (which implements greg) after call to emit_reload_insns,
this is how the rtl tree looks like:

(insn:HI 11 18 29 0 (set (subreg:SI (reg:V8HI 16 r16 [orig:118 i.0 ] [118]) 0)
        (mem:SI (lo_sum:SI (reg/f:SI 9 r9 [125])
                (symbol_ref:SI ("!D__i") [flags 0x2] <var_decl 0x41714480 i>)) [0 i+0 S4 A128])) 303 
{movsi_low} (insn_list 8 (ins\
n_list 18 (nil)))
    (nil))
...

r16 is set to a 16bit value here.

(insn 340 39 341 0 (set (reg:HI 16 r16)
        (reg:HI 145)) -1 (nil)
    (nil))
...


(insn:HI 82 80 83 0 (set (reg:HI 13 r13 [orig:181 i.0 ] [181])
        (subreg:HI (reg:V8HI 16 r16 [orig:118 i.0 ] [118]) 14)) 311 {*movhi_internal} (insn_list 17 (nil))
    (nil))
...

(insn:HI 121 152 122 2 (parallel [
            (set (subreg:SI (reg:V8HI 28 r28 [orig:121 T.6 ] [121]) 0)
                (and:SI (subreg:SI (reg:V8HI 16 r16 [orig:118 i.0 ] [118]) 0)
                    (subreg:SI (reg:V8HI 20 r20 [orig:119 j.1 ] [119]) 0)))
            (clobber (scratch:CC))
        ]) 81 {andsi3} (nil)
    (expr_list:REG_UNUSED (scratch:CC)
        (nil)))
==============================================

Later on insn 340 in above is changed to a load:

(insn 340 39 341 0 (set (reg:HI 16 r16)
        (mem:HI (plus:SI (reg/f:SI 1 r1)
                (const_int 78 [0x4e])) [0 S2 A8])) 311 {*movhi_internal} (nil)
    (nil))

....

Note that r16 expects the full 32-bit value. It only gets the 16bit value now.

(insn:HI 121 152 122 2 (parallel [
            (set (subreg:SI (reg:V8HI 28 r28 [orig:121 T.6 ] [121]) 0)
                (and:SI (subreg:SI (reg:V8HI 16 r16 [orig:118 i.0 ] [118]) 0)
                    (subreg:SI (reg:V8HI 20 r20 [orig:119 j.1 ] [119]) 0)))
            (clobber (scratch:CC))
        ]) 81 {andsi3} (nil)
    (expr_list:REG_UNUSED (scratch:CC)
        (nil)))
===============================================

Problem seems to originate in routine find_equiv_reg which decides that reg:HI r16 holds
the same value as reg:HI 145. But reg:SI r16 is referenced in and and:SI instruction later on.

-- 
           Summary: Bad code gen for vector code with -O3 (caused by
                    GRA/reload)
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: fjahanian at apple dot com
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: powerpc-apple-darwin7.2.0
  GCC host triplet: powerpc-apple-darwin7.2.0
GCC target triplet: powerpc-apple-darwin7.2.0


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-16 22:01 -------
Confirmed, related to bug 15289, in fact I would not doubt that this is a dup of that bug.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |rtl-optimization
     Ever Confirmed|                            |1
  GCC build triplet|powerpc-apple-darwin7.2.0   |
   GCC host triplet|powerpc-apple-darwin7.2.0   |
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2004-06-16 22:01:35
               date|                            |


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-06-16 22:33 -------
Is the call to find_equiv_reg you're talking about located in the inheritance
code from choose_reload_regs?  If so, that could be the same inheritance problem
I saw on i586: http://gcc.gnu.org/ml/gcc-patches/2004-04/msg01779.html


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-06-16 22:41 -------
As a side note, "GRA" doesn't sound much GCC-ish.  I think "greg" or "global"
would be more easily understood.


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-06-16 22:46 -------
Yes, senarios are similar. I will try the patch you mentioned and see how it goes.
P.S. will remeber to use greg.

- Thanks

-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-06-16 22:53 -------
Yup. This patch fixes my failure as well.


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-06-17 07:46 -------
> Yes, senarios are similar. I will try the patch you mentioned and see how it
> goes.

Thanks for the confirmation.  Hopefully with now two incarnations of the
problem, we'll able to come up with a solution.

Unfortunately, as pointed out by Joern
   http://gcc.gnu.org/ml/gcc-patches/2004-06/msg00061.html
my patch is wrong because it effectively disables inheritance (sorry for not
giving the link earlier, I didn't remember it wasn't reachable from the original
message).

As you can see, how reload inheritance is supposed to work is a little unclear.
 Its failure mode may be problematic or not.  I think a correct patch could be:

Index: reload1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload1.c,v
retrieving revision 1.436
diff -u -p -r1.436 reload1.c
--- reload1.c   20 May 2004 11:45:20 -0000      1.436
+++ reload1.c   17 Jun 2004 07:34:57 -0000
@@ -5654,10 +5654,9 @@ choose_reload_regs (struct insn_chain *c
                                                           i);
                    }

-                 if ((regs_used
-                      && ! free_for_value_p (regno, rld[r].mode,
-                                             rld[r].opnum, rld[r].when_needed,
-                                             rld[r].in, rld[r].out, r, 1))
+                 if (! free_for_value_p (regno, rld[r].mode,
+                                         rld[r].opnum, rld[r].when_needed,
+                                         rld[r].in, rld[r].out, r, 1)
                      || bad_for_class)
                    equiv = 0;
                }


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-06-17 16:39 -------
I looked at the follow-up thread and used the modified patch, very similar to yours.
FYI, this patch passed bootstrap and dejagnu tests on ppc-darwin. How ever, We are all still
concerned with performance implications of this change. I plan to run SPEC with this patch 
on a G5 machine and see if it shows any regressions.

RCS file: /cvs/gcc/gcc/gcc/reload1.c,v
retrieving revision 1.346.2.33.2.1
diff -c -p -r1.346.2.33.2.1 reload1.c
*** reload1.c   7 Jun 2004 17:05:08 -0000       1.346.2.33.2.1
--- reload1.c   17 Jun 2004 16:35:02 -0000
*************** choose_reload_regs (struct insn_chain *c
*** 5654,5664 ****
                                                           i);
                    }
  
!                 if ((regs_used
!                      && ! free_for_value_p (regno, rld[r].mode,
!                                             rld[r].opnum, rld[r].when_needed,
!                                             rld[r].in, rld[r].out, r, 1))
!                     || bad_for_class)
                    equiv = 0;
                }
  
--- 5654,5665 ----
                                                           i);
                    }
  
!                   if (bad_for_class
!                       || ! free_for_value_p (regno, rld[r].mode,
!                                              rld[r].opnum, rld[r].when_needed,
!                                              rld[r].in, rld[r].out, r, 1))
                    equiv = 0;
                }


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-06-18 18:18 -------
Ran SPEC benchmark on a 2 GHz G5 (apple-ppc-darwin). It showed *no* performance 
degradation. Eric, are you planning on submitting this patch for approval?

-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by GRA/reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-06-18 20:57 -------
> Ran SPEC benchmark on a 2 GHz G5 (apple-ppc-darwin). It showed *no* performance 
> degradation

That's good news.

> Eric, are you planning on submitting this patch for approval?

Joern was not very fond of it.  But I suppose it could be good enough.  I'll try
to submit it tomorrow.


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-06-23 07:10 -------
Patch submitted: http://gcc.gnu.org/ml/gcc-patches/2004-06/msg01647.html


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|ebotcazou at gcc dot gnu dot|
                   |org                         |
         AssignedTo|unassigned at gcc dot gnu   |ebotcazou at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
            Summary|Bad code gen for vector code|Bad code gen for vector code
                   |with -O3 (caused by         |with -O3 (caused by reload)
                   |GRA/reload)                 |


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by reload)

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2004-06-23 14:26 -------
Eric, which versions of GCC are affected by this problem? Is this a regression?

-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-06-23 14:43 -------
> Eric, which versions of GCC are affected by this problem?

Hard to say precisely.  I have an Ada testcase for the 3.4 branch, Fariborz's is
for mainline AFAICS.

> Is this a regression?

Ada bugs are not really considered in determining regressions so, if Fariborz
only sees it on mainline, let's target it to 3.5.0.


-- 


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

[Bug rtl-optimization/16028] Bad code gen for vector code with -O3 (caused by reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-09-30 21:44 -------
I think the best fix is http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00661.html
but it has been stuck in the review process for a while.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org
         AssignedTo|ebotcazou at gcc dot gnu dot|unassigned at gcc dot gnu
                   |org                         |dot org
             Status|ASSIGNED                    |NEW


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

[Bug rtl-optimization/16028] [4.0 Regression] Wrong code at -O3 (caused by reload)

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-11-22 08:48 -------
It is the cause of the following regressions on UltraSPARC 32-bit:
FAIL: gcc.c-torture/execute/simd-2.c execution,  -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/execute/simd-2.c execution,  -O3 -g

The failure looks very similar to the one in the vector code on PowerPC.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
 GCC target triplet|powerpc-apple-darwin7.2.0   |
            Summary|Bad code gen for vector code|[4.0 Regression] Wrong code
                   |with -O3 (caused by reload) |at -O3 (caused by reload)
   Target Milestone|---                         |4.0.0


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

[Bug rtl-optimization/16028] [4.0 Regression] Wrong code at -O3 (caused by reload)

[nathan at gcc dot gnu dot org]

------- Additional Comments From nathan at gcc dot gnu dot org  2004-12-02 12:06 -------
This appears fixed.  I wonder if 
2004-11-02  Nathan Sidwell  <nathan@codesourcery.com>

	PR rtl-optimization/17104
	* config/rs6000/rs6000.c (rs6000_emit_move): Don't wrap small
	loads in zero_extend.
fixed it. (I met a similar sounding problem on another port, where the
equivalent of the above did the trick).

-- 


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

[Bug rtl-optimization/16028] [4.0 Regression] Wrong code at -O3 (caused by reload)

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-12-05 05:38 -------
Actually I think this was fixed by PR 15289 so closing as a dup of that bug.

*** This bug has been marked as a duplicate of 15289 ***

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |DUPLICATE


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