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]

Committed: reload patch to prevent incorrect code generation


There are currently two problems in reload.

We are storing registers which may not be used for any reloads in
reload_reg_used.  Recently, we added a test against reload_reg_used at
the top of reload_reg_free_for_value_p.  Unfortunately, this has some
unwanted side effects.  Roughly, the following sequence of actions can
happen in choose_reload_regs:

  /* Allocate regs */
  for all reloads
     ...
     if we can find a register X to inherit from
       if (reload_reg_free_for_value_p (X, ...))
         {
	   mark_reload_reg_in_use (X, ...);
	 }
     ...
  /* Verify that we can really inherit in all cases where we thought
     we could.  */
  for all reloads
    if (inherited && ! reload_reg_free_for_value_p (...))
      disable the inheritance

The problem is that in the second call to reload_reg_free_for_value_p,
the reload's own register is in reload_reg_used, so the function will
return 0.  This causes needless pessimization of code.

The other bug is that when turning off a reload due to this bug, we
did not update reload_spill_index.  That caused the following piece of
code in reload_as_needed not to process the reload:
      /* The following if-statement was #if 0'd in 1.34 (or before...).
	 It's reenabled in 1.35 because supposedly nothing else
	 deals with this problem.  */

      /* If a register gets output-reloaded from a non-spill register,
         that invalidates any previous reloaded copy of it.
	 But forget_old_reloads_1 won't get to see it, because
	 it thinks only about the original insn.  So invalidate it here.  */

This causes us to generate incorrect code in situations like the following.
Before reload, we have
  (set (reg 47) (some_value))
  (set (reg 47) (ior (reg 47) (some other value)))
  (set (reg 48) (reg 47))
where reg 47 does not have a hard reg.  The first insn gets reloaded and
looks like
  (set (reg 1) (some_value))
  (set (stack slot for reg 47) (reg 1))
At this point, reg_last_reload_reg[47] == 1, so we think we can inherit
from reg 1 when reloading reg 47.  While reloading the second insn, both
reload bugs bite us: we end up deciding not to inherit from reg 1, so the
insn modifies the stack slot after reloading, and we fail to clear
reg_last_reload_reg.  The latter problem causes us to inherit the value
of reg 47 from reg 1 in the third insn, rather than reloading it from the
stack slot.

Unfortunately there are no known small testcases.  The bugs affect, among
other things, openssl.

The patch below, which I committed, fixes the incorrect code generation
problem.  It was reviewed by Joern Rennecke and approved by Jeff Law.
Another patch which hasn't been reviewed yet will fix the problem that
we're accidentally turning off perfectly good inheritance opportunitites.

Bernd

Index: ChangeLog
===================================================================
RCS file: /cvs/gcc/egcs/gcc/ChangeLog,v
retrieving revision 1.5350
diff -c -p -r1.5350 ChangeLog
*** ChangeLog	2000/01/05 07:10:33	1.5350
--- ChangeLog	2000/01/05 12:35:59
***************
*** 1,3 ****
--- 1,8 ----
+ 2000-01-05  Bernd Schmidt  <bernds@cygnus.co.uk>
+ 
+ 	* reload1.c (choose_reload_regs): When disabling a reload, also
+ 	set reload_spill_index to -1.
+ 
  2000-01-04  Joel Sherrill (joel@OARcorp.com>
  
  	* configure.in (m68*-*-rtemscoff*): New target, formal name for
Index: reload1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload1.c,v
retrieving revision 1.192
diff -c -p -r1.192 reload1.c
*** reload1.c	1999/12/24 14:04:53	1.192
--- reload1.c	2000/01/05 12:36:13
*************** choose_reload_regs (chain)
*** 5673,5678 ****
--- 5673,5679 ----
  	  clear_reload_reg_in_use (regno, rld[j].opnum,
  				   rld[j].when_needed, rld[j].mode);
  	rld[j].reg_rtx = 0;
+ 	reload_spill_index[j] = -1;
        }
  
    /* Record which pseudos and which spill regs have output reloads.  */


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