This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Patch: Bad insn: p789a.C, low opt levels, Alpha 32 (at least) (direct FR <-> GR move)


-- 

===================================================
Donn Terry                  mailto:donn@interix.com
Softway Systems, Inc.        http://www.interix.com
2850 McClelland Dr, Ste. 1800   Ft.Collins CO 80525
Tel: +1-970-204-9900           Fax: +1-970-204-9951
===================================================
Bad insn: p789a.C, low opt levels, Alpha 32 (at least) (direct FR <-> GR move)

Other architectures, particularly many versions of PA, may want to
consider a similar fix.

Problem:

The compiler fails with the following bad insn.  It is trying to
do a direct GR->FR move on the Alpha (for which there is no insn, let
alone instruction (in the absence of the CIX extension)).

(insn 585 16 18 (set (reg:SI 42 $f10)
        (subreg:SI (plus:DI (reg:DI 15 $15)
                (const_int 80 [0x50])) 0)) -1 (nil)
    (nil))

This is caused by the two calls to stupid_find_reg() in
stupid_life_analysis().  The first call fails if things are "bad
enough", which in this context means that the general registers
are all used up.  (Not surprising given inlining and using the
stupid allocator.)

The second call is attempted if there is an alternate register
class recorded as "worth trying", and it succeeds.  The alternate
class found is ALL_REGS, so the floating point registers (in this
case) become targets for a fixed point operation.  It should simply
have spilled the register to memory.

The alternate register class for the original pseudo is computed
in regclass(), which finds that for the long lifetime of this
register that the memory cost due to that lifetime overwhelms what
is supposed to be an expensive register-to-register move cost.

In this case, reg_alternate_class() for the original register
in the above insn (SI: 70) was computed to be ALL_REGS because
the cost of the move operation (90 in this case) was found to
be less than mem_cost (102).

The value of REGISTER_MOVE_COST for the FR<->GR move does correctly
reflect its cost:  it's a move through memory.

However it appears that there's always a pathalogical case such that
this can occur, particularly if we don't want to distort the cost
calculations elswhere.  There doesn't seem to be a way to say
to the register allocation code "this goes thru memory, so
if you're spilling, a register to register move for these types
is nonsense".

We need a way to disallow spill from using a register that's worse
than memory without distorting the other cost calculations.

Fix:

In parallel with, but much simpler than, FORBIDDEN_INC_DEC_CLASSES,
define FORBIDDEN_SPILL_MOVE, which the per-architecture .h file
can define if it can't possibly handle (or would never want to
handle) a spill move from a given register's mode into some register 
class.  In the case of the Alpha, this is quite simple: we never
want to allow ALL_REGS.  (Or, rather, only when the CIX extension
is present.)

I considered the alternative of using SECONDARY_MEMORY_NEEDED for this
function; it's not that difficult, but it seems safer to introduce the
new function for several reasons:

1) This doesn't impact architectures other than the Alpha unless/until
   they're ready for it.

2) I can imagine that it's possible SECONDARY_MEMORY_NEEDED could be true
   when spilling should go to the registers.  (There's some odd speed/space
   tradeoff.)

3) It might be that certain combinations of classes are "bad", and smart
   code could cause them to be avoidied (short of ALL_REGS).

   (These latter two fall into "avoiding failure-of-imagination", more than
   reflecting a known problem.)

ChangeLog:

Wed May 5 17:42:41 1999 Donn Terry (donn@interix.com)

	* config/alpha/alpha.h (FORBIDDEN_SPILL_MOVE): Define it.
	* regclass.c (regclass): Use FORBIDDEN_SPILL_MOVE.

diff -urP egcs.source.old/gcc/regclass.c egcs.source/gcc/regclass.c
--- egcs.source.old/gcc/regclass.c	Fri Apr  9 10:02:52 1999
+++ egcs.source/gcc/regclass.c	Wed May  5 10:13:29 1999
@@ -1087,6 +1087,10 @@ regclass (f, nregs)
 #ifdef FORBIDDEN_INC_DEC_CLASSES
 		  && ! (in_inc_dec[i] && forbidden_inc_dec_class[class])
 #endif
+#ifdef FORBIDDEN_SPILL_MOVE
+                  && ! FORBIDDEN_SPILL_MOVE (PSEUDO_REGNO_MODE (i),
+                        reg_class_subunion[(int) alt][class])
+#endif
 		  )
 		alt = reg_class_subunion[(int) alt][class];
 	  
diff -urP egcs.source.old/gcc/config/alpha/alpha.h egcs.source/gcc/config/alpha/alpha.h
--- egcs.source.old/gcc/config/alpha/alpha.h	Sun Apr 25 20:54:30 1999
+++ egcs.source/gcc/config/alpha/alpha.h	Wed May  5 10:13:18 1999
@@ -710,6 +710,13 @@ enum reg_class { NO_REGS, GENERAL_REGS, 
 #define REG_CLASS_CONTENTS	\
   { {0, 0}, {~0, 0x80000000}, {0, 0x7fffffff}, {~0, ~0} }
 
+/* The only path between GR and FR is via memory (in the absence of the
+   CIX extension).  Don't EVER allow the register assignment code to think 
+   that ALL_REGS is better than memory. */
+
+#define FORBIDDEN_SPILL_MOVE(REG, CLASS) \
+   (!TARGET_CIX && (CLASS) == ALL_REGS)
+
 /* The same information, inverted:
    Return the class number of the smallest class containing
    reg number REGNO.  This could be a conditional expression

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]