Bug 27117

This is why Changing reload is hard to do correctly :).

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...

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.

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);

Created attachment 11251 [details]
proposed patch

> -#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?

(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.

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.

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.

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

bug is latent again :-)

The latent bug is blocking a pretty serious (P2) 4.x regression.

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

(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 ? 

sure, and actually I have posted it to the GCC Patches mailing list to get approval for the target-independent part.

Created attachment 12108 [details]
patch that does not touch the middle-end

patch that does not touch the common parts of the compiler

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

fixed.