[Bug target/27117] New: gcc fails to build on sh64-elf targets

[Bug target/27117] New: gcc fails to build on sh64-elf targets

[kkojima at gcc dot gnu dot org]

The cross build for sh64-elf on x86 hosts fails during compilation
of libiberty/ramdom.c for 32bit SHmedia with

libiberty/random.c:362: internal compiler error: in push_reload, at
reload.c:1265

The following is a reduced testcase.  It ICEs with -O1 -m5-32media
on sh64-elf.

extern int tbl[32];
int n = 32;

int
foo (int idx)
{
  return tbl[idx % n];
}

Binary search shows that it starts to fail after the patch

r112637 | bonzini | 2006-04-03 20:20:07 +0900 (Mon, 03 Apr 2006) | 25 lines


-- 
           Summary: gcc fails to build on sh64-elf targets
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: kkojima at gcc dot gnu dot org
 GCC build triplet: i686-pc-linux-gnu
  GCC host triplet: i686-pc-linux-gnu
GCC target triplet: sh64-elf


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2006-04-11 17:46 -------
This is why Changing reload is hard to do correctly :).


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bonzini at gnu dot org
            Summary|gcc fails to build on sh64- |[4.2 Regression] gcc fails
                   |elf targets                 |to build on sh64-elf targets
   Target Milestone|---                         |4.2.0


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[bonzini at gnu dot org]

-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |bonzini at gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-04-12 07:11:15
               date|                            |


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[bonzini at gnu dot org]

------- Comment #2 from bonzini at gnu dot org  2006-04-12 07:30 -------
And the funny part is, it is again Dale's patch that causes the failure.

I'm more and more inclined to revert that part, but it may be a latent bug as
in the AIX case (note: David Edelsohn decided to "fix" the bug by making sure
the problematic RTL is not produced, but it still was an rs6000 latent bug in
some sense).  So, if Joern wants to take a look...


-- 

bonzini at gnu dot org changed:

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


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[amylaar at gcc dot gnu dot org]

------- Comment #3 from amylaar at gcc dot gnu dot org  2006-04-12 13:46 -------
sh64 has indexed addressing, but the addition is always done as 64 bit,
and there are currently no implemenmtations that allow the 64 bit logical
address space to be re-mapped into a 32 physical address space - instead
they trap on any access to an address that does not fit into a 32 bit
address space.

This makes using indexed addressing with Pmode for -m5-32media (where Pmode
is SImode) unsafe, since some optimizations can fold additions into indexed
addressing and thus cause out-of-range addresses.  Therefore, INDEX_REG_CLASS
is NO_REGS for -m5-32media.

The division code produces an address with a DImode plus of two registers -
this is safe, since we exactly describe what the hardware does.
However, find_reloads_address_1 sees a plus and recurses with CONTEXT set to 1,
and then uses INDEX_REG_CLASS; it does not take into account that the mode
is not Pmode.

I think the best solution is to have an INDEX_REG_CLASS_FOR_MODE macro,
which defaults to INDEX_REG_CLASS.  Then this macro can be defined for the
SH to return GENERAL_REGS for DImode when compiling SHmedia code.

A kludgy solution would be to make reload reload the sum into a base register
(to cover the general case), and make the SH LEGITIMIZE_RELOAD_ADDRESS
recognize a sum with a non-pmode PLUS, and only reload pseudos inside into
GENERAL_REGS.


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[paolo dot bonzini at lu dot unisi dot ch]

------- Comment #4 from paolo dot bonzini at lu dot unisi dot ch  2006-04-12 14:09 -------
Subject: Re:  [4.2 Regression] gcc fails to build on sh64-elf
 targets


> I think the best solution is to have an INDEX_REG_CLASS_FOR_MODE macro,
> which defaults to INDEX_REG_CLASS.  Then this macro can be defined for the
> SH to return GENERAL_REGS for DImode when compiling SHmedia code.
>   
Thanks for the analysis.  I quickly tested that your approach works for 
Kaz's testcase.  However I don't feel confident enough to write this 
patch though -- and even less to document it.

Are you going to do it,  or should I go on and revert the regclass.c change?

This is what I tested BTW:

Index: reload.c
===================================================================
--- reload.c    (revision 112658)
+++ reload.c    (working copy)
@@ -5316,7 +5316,7 @@ find_reloads_address_1 (enum machine_mod
   RTX_CODE code = GET_CODE (x);

   if (context == 1)
-    context_reg_class = INDEX_REG_CLASS;
+    context_reg_class = GET_MODE (x) == DImode ? GENERAL_REGS : 
INDEX_REG_CLASS;
   else
     context_reg_class = base_reg_class (mode, outer_code, index_code);


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[amylaar at gcc dot gnu dot org]

------- Comment #5 from amylaar at gcc dot gnu dot org  2006-04-12 19:59 -------
Created an attachment (id=11251)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=11251&action=view)
proposed patch


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[kkojima at gcc dot gnu dot org]

------- Comment #6 from kkojima at gcc dot gnu dot org  2006-04-12 21:43 -------
> -#define INDEX_REG_CLASS \
> -  (!ALLOW_INDEXED_ADDRESS ? NO_REGS : TARGET_SHMEDIA ? GENERAL_REGS : R0_REGS)
> +#define INDEX_REG_CLASS_FOR_MODE(MODE) \
> +  ((MODE) == DImode && TARGET_SHMEDIA ? 1 \

Should this last 1 be GENERAL_REGS?


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[amylaar at gcc dot gnu dot org]

------- Comment #7 from amylaar at gcc dot gnu dot org  2006-04-13 11:45 -------
(In reply to comment #6)
> > -#define INDEX_REG_CLASS \
> > -  (!ALLOW_INDEXED_ADDRESS ? NO_REGS : TARGET_SHMEDIA ? GENERAL_REGS : R0_REGS)
> > +#define INDEX_REG_CLASS_FOR_MODE(MODE) \
> > +  ((MODE) == DImode && TARGET_SHMEDIA ? 1 \
> 
> Should this last 1 be GENERAL_REGS?
> 

Oops, yes.


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[kkojima at gcc dot gnu dot org]

------- Comment #8 from kkojima at gcc dot gnu dot org  2006-04-13 14:00 -------
I've tested Joern's patch with #7 tweak on i686-pc-linux-gnu,
x86 cross sh{64,4}-unknown-linux-gnu and x86 cross sh64-elf targets.
There are no bootstrap or build problems and the results of
regression tests look fine on all targets.


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[bonzini at gnu dot org]

------- Comment #9 from bonzini at gnu dot org  2006-04-18 08:13 -------
I'm reverting the PR19653 regclass.c patch for now.  Joern of course if you
want to post your patch for testing, it'll help reinstating the patch in 4.3.


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[bonzini at gcc dot gnu dot org]

------- Comment #10 from bonzini at gnu dot org  2006-04-18 08:23 -------
Subject: Bug 27117

Author: bonzini
Date: Tue Apr 18 08:23:39 2006
New Revision: 113026

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113026
Log:
2006-04-18  Paolo Bonzini  <bonzini@gnu.org>

        PR target/27117

        Partial revert of revision 112637
        2006-04-03  Paolo Bonzini  <bonzini@gnu.org>
                    Dale Johannesen  <dalej@apple.com>

        PR target/19653
        * regclass.c (struct reg_pref): Update documentation.
        (regclass): Set prefclass to NO_REGS if memory is the best option.
        (record_reg_classes): Cope with a prefclass set to NO_REGS.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/regclass.c


-- 


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

[Bug target/27117] [4.2 Regression] gcc fails to build on sh64-elf targets

[bonzini at gnu dot org]

------- Comment #11 from bonzini at gnu dot org  2006-04-18 13:26 -------
bug is latent again :-)


-- 

bonzini at gnu dot org changed:

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


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

[Bug target/27117] SH backend cheats to reload -- disables indexed addressing but uses it internally

[bonzini at gnu dot org]

------- Comment #12 from bonzini at gnu dot org  2006-08-07 08:16 -------
The latent bug is blocking a pretty serious (P2) 4.x regression.

"Lie to reload, and it will take its revenge."


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |26778
              nThis|                            |
             Status|RESOLVED                    |REOPENED
           Keywords|ice-on-valid-code           |
         Resolution|FIXED                       |
            Summary|[4.2 Regression] gcc fails  |SH backend cheats to reload
                   |to build on sh64-elf targets|-- disables indexed
                   |                            |addressing but uses it
                   |                            |internally


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

[Bug target/27117] SH backend cheats to reload -- disables indexed addressing but uses it internally

[amylaar at gcc dot gnu dot org]

------- Comment #13 from amylaar at gcc dot gnu dot org  2006-08-07 10:28 -------
(In reply to comment #12)
> The latent bug is blocking a pretty serious (P2) 4.x regression.
> 
> "Lie to reload, and it will take its revenge."

Have you tried the patch from comments 5 to 8 ? 


-- 


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

[Bug target/27117] SH backend cheats to reload -- disables indexed addressing but uses it internally

[bonzini at gnu dot org]

------- Comment #14 from bonzini at gnu dot org  2006-08-07 11:11 -------
sure, and actually I have posted it to the GCC Patches mailing list to get
approval for the target-independent part.


-- 


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

[Bug target/27117] SH backend cheats to reload -- disables indexed addressing but uses it internally

[bonzini at gnu dot org]

------- Comment #15 from bonzini at gnu dot org  2006-08-21 06:20 -------
Created an attachment (id=12108)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12108&action=view)
patch that does not touch the middle-end

patch that does not touch the common parts of the compiler


-- 


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

[Bug target/27117] SH backend cheats to reload -- disables indexed addressing but uses it internally

[bonzini at gcc dot gnu dot org]

------- Comment #16 from bonzini at gnu dot org  2006-09-07 08:19 -------
Subject: Bug 27117

Author: bonzini
Date: Thu Sep  7 08:19:32 2006
New Revision: 116746

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116746
Log:
2006-09-07  Paolo Bonzini  <bonzini@gnu.org>

        PR target/27117
        * config/sh/sh.md (divsi_inv_qitable, divsi_inv_hitable): New patterns.
        (divsi_inv_m1): Use them.
        (UNSPEC_DIV_INV_TABLE): New constant.


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


-- 


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

[Bug target/27117] SH backend cheats to reload -- disables indexed addressing but uses it internally

[bonzini at gnu dot org]

------- Comment #17 from bonzini at gnu dot org  2006-09-07 08:20 -------
fixed.


-- 

bonzini at gnu dot org changed:

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


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