[Bug target/44707] New: operand requires impossible reload

[Bug target/44707] New: operand requires impossible reload

[jakub at gcc dot gnu dot org]

extern struct { int a, b, c, d; } v;
extern int w;

void
foo (void)
{
  int e1 = v.a;
  int e2 = w;
  int e3 = v.b;
  int e4 = v.c;
  int e5 = v.d;
  __asm__ volatile ("/* %0 %1 %2 %3 %4 */" : : "nro" (e1), "nro" (e2), "nro"
(e3), "nro" (e4), "nro" (e5));
}

with -O2 -m32 gives:
xlog.i: In function 'foo':
xlog.i:12:3: error: 'asm' operand requires impossible reload
xlog.i:12:3: error: 'asm' operand requires impossible reload

With "nro" is replaced with "nrm", it compiles fine (but, GCC except for latest
trunk might put there side-effects into the unused operand).  With "nrm<>"
(which is wrong in this case, the asm would actually need to do ld%U1 %0,%1 or
something similar, but then would be valid) it dies with similar error.

It seems that before reload only when "nrm" is used the asm has (mem (lo_sum
(reg) (symbol_ref))) or (mem (plus (reg) (const_int))) but with "nro" or
"nrm<>" it contains bare (mem (symbol_ref)) or (mem (plus (symbol_ref)
(const_int))) and apparently reload isn't able to deal with that.  Not sure if
it is a target powerpc bug, or reload bug, or both.  In any case, there
shouldn't be anything impossible here to reload.
I see the ICEs with GCC 4.1, 4.4 and 4.6, so perhaps it is not a regression.


-- 
           Summary: operand requires impossible reload
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
GCC target triplet: powerpc-linux


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

[Bug target/44707] operand requires impossible reload

[uweigand at gcc dot gnu dot org]

------- Comment #1 from uweigand at gcc dot gnu dot org  2010-06-29 16:56 -------
I agree, this looks like a longstanding bug in
rs6000_legitimize_reload_address.

What happens here is that find_reloads is called on this insn:

(insn 15 8 18 2 pr44707.c:13 (asm_operands/v ("/* %0 %1 %2 %3 %4 */") ("") 0 [
            (mem/s/c:SI (symbol_ref:SI ("v") [flags 0xc0] <var_decl 0xf6f90fc0
v>) [3 v.a+0 S4 A32])
            (mem/c/i:SI (symbol_ref:SI ("w") [flags 0xc4] <var_decl 0xf6f91020
w>) [3 w+0 S4 A32])
            (mem/s/c:SI (const:SI (plus:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>)
                        (const_int 4 [0x4]))) [3 v.b+0 S4 A32])
            (mem/s/c:SI (const:SI (plus:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>)
                        (const_int 8 [0x8]))) [3 v.c+0 S4 A32])
            (mem/s/c:SI (const:SI (plus:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>)
                        (const_int 12 [0xc]))) [3 v.d+0 S4 A32])
        ]
         [
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
        ]
         [] pr44707.c:14) -1 (nil))

rs6000_find_reloads_address notices that it can rewrite
  (symbol_ref:SI "v")
to
  (lo_sum:SI (high:SI (symbol_ref:SI "v")) (symbol_ref:SI "v"))
(and place a reload on the (high:SI) subexpression) and does so.  This change
remains in the insn, and when in the next iteration find_reloads is called
again, the insn now looks like:

(insn 15 8 18 2 pr44707.c:13 (asm_operands/v ("/* %0 %1 %2 %3 %4 */") ("") 0 [
            (mem/s/c:SI (lo_sum:SI (high:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>))
                    (symbol_ref:SI ("v") [flags 0xc0] <var_decl 0xf6f90fc0 v>))
[3 v.a+0 S4 A32])
            (mem/c/i:SI (lo_sum:SI (high:SI (symbol_ref:SI ("w") [flags 0xc4]
<var_decl 0xf6f91020 w>))
                    (symbol_ref:SI ("w") [flags 0xc4] <var_decl 0xf6f91020 w>))
[3 w+0 S4 A32])
            (mem/s/c:SI (const:SI (plus:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>)
                        (const_int 4 [0x4]))) [3 v.b+0 S4 A32])
            (mem/s/c:SI (const:SI (plus:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>)
                        (const_int 8 [0x8]))) [3 v.c+0 S4 A32])
            (mem/s/c:SI (const:SI (plus:SI (symbol_ref:SI ("v") [flags 0xc0]
<var_decl 0xf6f90fc0 v>)
                        (const_int 12 [0xc]))) [3 v.d+0 S4 A32])
        ]
         [
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
            (asm_input:SI ("nro") (null):0)
        ]
         [] pr44707.c:14) -1 (nil))

However, this expression is now no longer recognized by
rs6000_legitimize_reload_address, and therefore no reload on (high:SI) is
pushed.

Thus, when the reload is finally processed, a reload insn like this is
generated:

(insn 26 8 27 2 pr44707.c:13 (set (reg:SI 10 10)
        (lo_sum:SI (high:SI (symbol_ref:SI ("v") [flags 0xc0] <var_decl
0xf6f90fc0 v>))
            (symbol_ref:SI ("v") [flags 0xc0] <var_decl 0xf6f90fc0 v>))) -1
(nil))

As this does not actually correspond to any valid pattern, an assertion is
triggered.

The underlying problem is that with the current reload setup, an implementation
of LEGITIMATE_RELOAD_ADDRESS must always recognize expressions it has itself
generated in an earlier call.  And indeed, that's what a comment in
rs6000_legitimize_reload_address says:

static rtx
rs6000_legitimize_reload_address (rtx x, enum machine_mode mode,
                                  int opnum, int type,
                                  int ind_levels ATTRIBUTE_UNUSED, int *win)
{
  bool reg_offset_p = reg_offset_addressing_ok_p (mode);

  /* We must recognize output that we have already generated ourselves.  */
  if (GET_CODE (x) == PLUS
      && GET_CODE (XEXP (x, 0)) == PLUS
      && GET_CODE (XEXP (XEXP (x, 0), 0)) == REG
      && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT
      && GET_CODE (XEXP (x, 1)) == CONST_INT)


However, this recognizes only certain types of such output, and in particular
not the one that shows up in this test case.

It seems to me that simply extending rs6000_legitimate_reload_address to handle
this case as well should fix the bug.


-- 

uweigand 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         |2010-06-29 16:56:47
               date|                            |


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

[Bug target/44707] operand requires impossible reload

[uweigand at gcc dot gnu dot org]

------- Comment #2 from uweigand at gcc dot gnu dot org  2010-06-29 17:03 -------
Created an attachment (id=21041)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21041&action=view)
Recognize (lo_sum (high ...) ...) in rs6000_legitimize_reload_address

> It seems to me that simply extending rs6000_legitimate_reload_address to handle
> this case as well should fix the bug.

And indeed, this (otherwise untested) patch fixes the bug for me.


-- 


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

[Bug target/44707] operand requires impossible reload

[uweigand at gcc dot gnu dot org]

-- 

uweigand at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |uweigand at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2010-06-29 16:56:47         |2010-07-01 19:07:33
               date|                            |


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

[Bug target/44707] operand requires impossible reload

[uweigand at gcc dot gnu dot org]

------- Comment #3 from uweigand at gcc dot gnu dot org  2010-07-01 19:14 -------
Patch posted here:
http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00082.html


-- 


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

[Bug target/44707] operand requires impossible reload

[uweigand at gcc dot gnu dot org]

------- Comment #4 from uweigand at gcc dot gnu dot org  2010-07-02 11:48 -------
Subject: Bug 44707

Author: uweigand
Date: Fri Jul  2 11:48:30 2010
New Revision: 161703

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161703
Log:
ChangeLog:

        PR target/44707
        * config/rs6000/rs6000.c (rs6000_legitimize_reload_address): Recognize
        (lo_sum (high ...) ...) patterns generated by earlier passes.

testsuite/ChangeLog:

        PR target/44707
        * gcc.c-torture/compile/pr44707.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr44707.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/44707] operand requires impossible reload

[uweigand at gcc dot gnu dot org]

------- Comment #5 from uweigand at gcc dot gnu dot org  2010-07-02 11:50 -------
Fixed.


-- 

uweigand at gcc dot gnu dot org changed:

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


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

[Bug target/44707] operand requires impossible reload

[ebotcazou at gcc dot gnu dot org]

------- Comment #6 from ebotcazou at gcc dot gnu dot org  2010-07-25 21:46 -------
Subject: Bug 44707

Author: ebotcazou
Date: Sun Jul 25 21:46:32 2010
New Revision: 162521

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162521
Log:
        PR target/44707
        * config/sparc/sparc-protos.h (sparc_legitimize_reload_address): New.
        * config/sparc/sparc.c: Include reload.h.
        (legitimize_tls_address): Rename into...
        (sparc_legitimize_tls_address): ...this.
        (legitimize_pic_address): Rename into...
        (sparc_legitimize_pic_address): ...this.
        (sparc_expand_move): Adjust to above renaming.
        (sparc_tls_referenced_p): Likewise.
        (sparc_legitimize_tls_address): Likewise.
        (sparc_legitimize_pic_address): Likewise.
        (sparc_legitimize_address): Likewise.
        (sparc_output_mi_thunk): Likewise.
        (sparc_legitimize_reload_address): New global function.  Recognize
        (lo_sum (high ...) ...) patterns generated by earlier passes.
        * config/sparc/sparc.h (LEGITIMIZE_RELOAD_ADDRESS): Use above function.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sparc/sparc-protos.h
    trunk/gcc/config/sparc/sparc.c
    trunk/gcc/config/sparc/sparc.h


-- 


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