[Bug rtl-optimization/34529] New: Wrong code with altivec and offsets

[Bug rtl-optimization/34529] New: Wrong code with altivec and offsets

[pinskia at gcc dot gnu dot org]

Testcase:
vector float b[560560];
vector float c[560560];
void f(void);
vector float Mult(vector float a)
{
  b[560560/16] = a;
  asm
("":::"memory","0","3","4","5","6","7","8","9","10","11","12","14","15","16","17","18","19","20","21","22","23","24","25","26","27","28","29");
  b[560560/16] = a;
}

---- CUT ----
We get:
        lis 9,0x8
        ori 9,9,36272
        add 9,9,9
        stvx 0,0,9

Which is obviously wrong.


-- 
           Summary: Wrong code with altivec and offsets
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pinskia at gcc dot gnu dot org
GCC target triplet: powerpc64-linux-gnu


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

[Bug rtl-optimization/34529] Wrong code with altivec and offsets

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2007-12-19 00:23 -------
I forgot to say compile this code with -maltivec -include altivec.h -O2.


-- 


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

[Bug rtl-optimization/34529] Wrong code with altivec stores and offsets

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2007-12-19 00:25 -------
After reload we have:
(insn 35 10 37 2 t.c:9 (set (reg:SI 9 9)
        (reg/f:SI 65 lr [122])) 325 {*movsi_internal1} (nil))

(insn 37 35 38 2 t.c:9 (set (reg:SI 9 9)
        (const_int 560560 [0x88db0])) 325 {*movsi_internal1} (nil))

(insn 38 37 13 2 t.c:9 (set (reg:SI 9 9)
        (plus:SI (reg:SI 9 9)
            (reg:SI 9 9))) 79 {*addsi3_internal1} (expr_list:REG_EQUIV (plus:SI
(reg:SI 9 9)
            (const_int 560560 [0x88db0]))
        (nil)))

(insn:HI 13 38 16 2 t.c:9 (set (mem/s:V4SF (reg:SI 9 9) [3 b+560560 S16 A128])
        (reg/v:V4SF 77 0 [orig:121 a ] [121])) 655 {altivec_stvx_v4sf} (nil))


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ra


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

[Bug rtl-optimization/34529] Wrong code with altivec stores and offsets

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2007-12-19 00:35 -------
Note here is a better testcase which clobbers all the usable registers:
static vector float b[560560];
void f(void);
vector float Mult(vector float a)
{
  b[560560/16] = a;
  asm
("":::"memory","0","3","4","5","6","7","8","9","10","11","12","14","15","16","17","18","19","20","21","22","23","24","25","26","27","28","29",
"lr", "30", "31", "ctr");
  b[560560/16] = a;
}


-- 


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

[Bug rtl-optimization/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[pinskia at gcc dot gnu dot org]

------- Comment #4 from pinskia at gcc dot gnu dot org  2007-12-19 01:28 -------
4.0.2 works:
        ld 9,.LC1@toc(2)
        li 11,-29264
        stvx 2,9,11

In 32bit mode for 4.0.3:

        lis 9,b+589824@ha
        la 9,b+589824@l(9)
        li 11,-29264
        stvx 0,9,11

32bits, 4.1.0:
        lis 9,0x8
        ori 9,9,36272
        add 9,9,9
        stvx 2,0,9


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical
 GCC target triplet|powerpc64-linux-gnu         |powerpc*-linux-gnu
      Known to fail|                            |4.1.1 4.2.0 4.3.0
      Known to work|                            |4.0.3
            Summary|Wrong code with altivec     |[4.1/4.2/4.3 Regression]
                   |stores and offsets          |Wrong code with altivec
                   |                            |stores and offsets
   Target Milestone|---                         |4.1.3


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

[Bug rtl-optimization/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2007-12-19 23:01 -------
I should note this is a reduced testcase from a much bigger testcase and that I
could not provide that non reduced testcase as it is part of some PS3 video
game.


-- 


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

[Bug rtl-optimization/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2007-12-20 02:08 -------
This was introduced by the change to rs6000_legitimate_offset_address_p which
was done by:
        PR target/18506
        * config/rs6000/altivec.md (vec_init<mode>): New.
        (vec_set<mode>): New.
        (vec_extract<mode>): New.
        * config/rs6000/rs6000.c (rs6000_expand_vector_init): New.
        (rs6000_expand_vector_set): New.
        (rs6000_expand_vector_extract): New.
        (rs6000_legitimate_offset_address_p): Offset addresses are valid
        for Altivec modes before reload.
        (altivec_expand_vec_init_builtin): New.
        (get_element_number): New.
        (altivec_expand_vec_set_builtin): New.
        (altivec_expand_vec_ext_builtin): New.
        (altivec_expand_builtin): Expand vec_init, vec_set, and vec_ext
        builtins.
        (altivec_init_builtins): Init vec_init, vec_set, and vec_ext
        builtins.
        * config/rs6000/rs6000.h (rs6000_builtins): Add
        ALTIVEC_BUILTIN_VEC_INIT_<mode>, ALTIVEC_BUILTIN_VEC_SET_<mode>,
        ALTIVEC_BUILTIN_VEC_EXT_<mode>.
        * config/rs6000/rs6000-protos.h: Declare new functions.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dje at gcc dot gnu dot org


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

[Bug rtl-optimization/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[dje at gcc dot gnu dot org]

------- Comment #7 from dje at gcc dot gnu dot org  2007-12-21 14:36 -------
When reload converts the offset address to an indexed address, it re-uses the
same register to which the global variable symbol was assigned.


-- 

dje at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-12-21 14:36:20
               date|                            |


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

[Bug rtl-optimization/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[uweigand at gcc dot gnu dot org]

------- Comment #8 from uweigand at gcc dot gnu dot org  2008-01-09 19:23 -------
This is a long-standing problem in gen_reload.  This routine fundamentally
assumes that every PLUS expression that describes a legitimate address can
be reloaded into a register without requiring any additional scratch registers.

It first attempts to generate a simple (set (...) (plus ...)) pattern.  If
the target does not accept this pattern, it tries as a fall-back to generate
a two-insn sequence to perform the load.

This code looks fundamentally broken to me, in that it does not check whether
the first of those two insns clobbers a register still required by the second.
However, even if such a check were added, at this point we could not actually
solve the problem, because it's far too late to allocate another register.

I ran into the same problem in the s390 back-end a long time ago, and solved
it by providing a back-end specific secondary reload rule (requiring a scratch
register) to reload PLUS patterns that require multiple instructions.

I think you'll have to do the same for rs6000.

(As a separate issue, the broken code in gen_reload should be replaced by an
abort -- at least back-ends where the secondary reload is missing would no
longer generate wrong code in that case.)


-- 


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

[Bug target/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[ebotcazou at gcc dot gnu dot org]

------- Comment #9 from ebotcazou at gcc dot gnu dot org  2008-01-11 08:57 -------
Recategorizing as per Ulrich's analysis.


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org
          Component|rtl-optimization            |target


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

[Bug target/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[dje at gcc dot gnu dot org]

------- Comment #10 from dje at gcc dot gnu dot org  2008-01-21 18:18 -------
The secondary_reload target hook does not provide the full MEM RTL as an
argument, only the address RTL, which prevents the hook implementation from
distinguishing ALTIVEC, which only allows indexed addresses, from GPR and FPR,
which allow offset addresses.

If the target hook can add a scratch register, reload should be able to add a
scratch register itself.

If reload cannot fix this address correctly, rs6000_legitimate_offset_address
needs to reject offset addresses for Altivec modes.


-- 


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

[Bug target/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[uweigand at gcc dot gnu dot org]

------- Comment #11 from uweigand at gcc dot gnu dot org  2008-01-21 18:54 -------
The secondary reload hook does not need to make the decision whether or
not indexed addresses are allowed; that decision has already been taken.

The purpose of the secondary reload hook is simply to do whatever it takes
to load an indexed address into a base register (after reload has already
decided that this address needs to be loaded).

Reload does not allocate scratch registers always since it assumes that
targets provide a "load address" instruction that it able to perform
this operation without scratch register.  You need the secondary reload
to let common code know that this doesn't work on your target.


-- 


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

[Bug target/34529] [4.1/4.2/4.3 Regression] Wrong code with altivec stores and offsets

[dje at gcc dot gnu dot org]

------- Comment #12 from dje at gcc dot gnu dot org  2008-01-22 21:12 -------
Subject: Bug 34529

Author: dje
Date: Tue Jan 22 21:12:05 2008
New Revision: 131741

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131741
Log:
        PR target/34529
        * config/rs6000/rs6000.c (rs6000_legitimate_offset_address_p):
        Offset addresses are not valid for Altivec or paired float modes.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c


-- 


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

[Bug target/34529] [4.1/4.2 Regression] Wrong code with altivec stores and offsets

[jakub at gcc dot gnu dot org]

------- Comment #13 from jakub at gcc dot gnu dot org  2008-01-22 23:41 -------
Fixed on the trunk.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|4.1.1 4.2.0 4.3.0           |4.1.1 4.2.0
      Known to work|4.0.3                       |4.0.3 4.3.0
            Summary|[4.1/4.2/4.3 Regression]    |[4.1/4.2 Regression] Wrong
                   |Wrong code with altivec     |code with altivec stores and
                   |stores and offsets          |offsets


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

[Bug target/34529] [4.2 Regression] Wrong code with altivec stores and offsets

[jsm28 at gcc dot gnu dot org]

------- Comment #14 from jsm28 at gcc dot gnu dot org  2008-07-04 22:26 -------
Closing 4.1 branch.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.1/4.2 Regression] Wrong  |[4.2 Regression] Wrong code
                   |code with altivec stores and|with altivec stores and
                   |offsets                     |offsets
   Target Milestone|4.1.3                       |4.2.5


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

[Bug target/34529] [4.2 Regression] Wrong code with altivec stores and offsets

[jsm28 at gcc dot gnu dot org]

------- Comment #15 from jsm28 at gcc dot gnu dot org  2009-03-31 14:56 -------
Closing 4.2 branch, fixed in 4.3.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to fail|4.1.1 4.2.0                 |4.1.1 4.2.0 4.2.5
         Resolution|                            |FIXED
   Target Milestone|4.2.5                       |4.3.0


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