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]
Other format: [Raw text]

[patch] PR target/36613: reload codegen bug


Hi,

reload has a problem when it tries to reuse (not to be confused with 
inheriting) a reload with a different mode.  In the problematic case at 
hand we first find a SImode reload of %ecx to %edx (using %ecx as 
reload_reg), and try to fit a second reload from %cl to %dl into that.  
This is quite fine, just that the merging of both reloads overwrites the 
in/out members, and leaves us with a QImode reload.  Or better, it merges 
the inmode/outmode (depending on size, i.e. leaves the larger), but 
overwrites the reload operands itself.

So we are left with a SImode reload dealing with %cl and %dl.  
Unfortunately do_input_reload (and do_output_reload) don't care that much 
for inmode/outmode (which would be correct here), but rather simply take 
what is noted in in/out.  So we emit a QImode move, thereby not correctly 
emitting the insns for the first reload.

As I noted in the large bugzilla comment I see two ways of fixing this:
1) also take the size of operands into account while merging the in/out 
   members, in push_reload
2) overhaul do_input_reload and do_output_reload to care for 
   inmode/outmode _first_ before falling back to the operands.

Option (2) is more complicated as it potentially requires adjusting the 
RTXes we find in in/out for the mode we want.  There's also a large scary 
comment at do_input_reload speculating about all kinds of potential 
breakages, it's very old and probably half of the fear in there doesn't 
apply anymore, but I didn't feel like poking into 20 year old history to 
see why and how it mattered exactly.  I think the whole comment is mood 
when we rely on inmode/outmode, but in the end I only did (1), found it to 
fix the testcase and regstrapped this successfully on i686 and x86_64.

Since that I somewhat changed the patch, so I'm currently regstrapping it 
again.  Okay for trunk if that passes?  At least 4.3 needs that too, 
eventually.  If someone also wants to test this on 4.2 I would be glad.


Ciao,
Michael.

	PR target/36613

	* reload.c (push_reload): Merge in,out,in_reg,out_reg members
	for reused reload, instead of overwriting them.

	* gcc.target/i386/pr36613.c: New testcase.

Index: gcc/reload.c
===================================================================
*** gcc/reload.c	(revision 137652)
--- gcc/reload.c	(working copy)
*************** push_reload (rtx in, rtx out, rtx *inloc
*** 1403,1415 ****
  	      else
  		remove_address_replacements (rld[i].in);
  	    }
! 	  rld[i].in = in;
! 	  rld[i].in_reg = in_reg;
  	}
        if (out != 0)
  	{
! 	  rld[i].out = out;
! 	  rld[i].out_reg = outloc ? *outloc : 0;
  	}
        if (reg_class_subset_p (class, rld[i].class))
  	rld[i].class = class;
--- 1403,1438 ----
  	      else
  		remove_address_replacements (rld[i].in);
  	    }
! 	  /* When emitting reloads we don't necessarily look at the in-
! 	     and outmode, but also directly at the operands (in and out).
! 	     So we can't simply overwrite them with whatever we have found
! 	     for this (to-be-merged) reload, we have to "merge" that too.
! 	     Reusing another reload already verified that we deal with the
! 	     same operands, just possibly in different modes.  So we
! 	     overwrite the operands only when the new mode is larger.
! 	     See also PR33613.  */
! 	  if (!rld[i].in
! 	      || GET_MODE_SIZE (GET_MODE (in))
! 	           > GET_MODE_SIZE (GET_MODE (rld[i].in)))
! 	    rld[i].in = in;
! 	  if (!rld[i].in_reg
! 	      || (in_reg
! 		  && GET_MODE_SIZE (GET_MODE (in_reg))
! 	             > GET_MODE_SIZE (GET_MODE (rld[i].in_reg))))
! 	    rld[i].in_reg = in_reg;
  	}
        if (out != 0)
  	{
! 	  if (!rld[i].out
! 	      || (out
! 		  && GET_MODE_SIZE (GET_MODE (out))
! 	             > GET_MODE_SIZE (GET_MODE (rld[i].out))))
! 	    rld[i].out = out;
! 	  if (outloc
! 	      && (!rld[i].out_reg
! 		  || GET_MODE_SIZE (GET_MODE (*outloc))
! 		     > GET_MODE_SIZE (GET_MODE (rld[i].out_reg))))
! 	    rld[i].out_reg = *outloc;
  	}
        if (reg_class_subset_p (class, rld[i].class))
  	rld[i].class = class;


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