Re: [PATCH] Fix PR37053: Move tweaks of commutative precedence to target hook

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Peter Bergner wrote:
> On Tue, 2009-07-14 at 23:09 +0200, Paolo Bonzini wrote:
> > What about this?  A bit simpler. :-)
> > 
> > (And this time I tested it with the right options).
> 
> This bootstrapped and regtested with no regressions on powerpc64-linux,
> as did Richard's patch.
> 
> I have to agree with Richard, that it seems a little hackish setting 
> reload_completed this way.

Peter asked me to have a look at this problem as well.  I agree that
setting reload_completed here seems a little dangerous.  In particular,
because reload is in fact *not* completed: in particulr, replacement of
pseudo registers by their permanent stack slots has not yet been
performed.  Code that assumes "reload_completed" means that no pseudos
occur in instructions may well get confused by this ...

I'm wondering if we cannot make a simple local fix to the problem
that "validate_replace_rtx" cannot be completely undone by doing
another "validate_replace_rtx".  We should be able to keep the
logic in reload_as_needed as-is, but simply use the existing
change-group management routines to cancel or confirm the change
after we did the extra test ...

The patch below implements this option.  Any comments?  Can you
verify this fixes the original problem?

Bye,
Ulrich

ChangeLog:

	* reload1.c (reload_as_needed): Use cancel_changes to completely
	undo a failed replacement attempt.


Index: gcc/reload1.c
===================================================================
*** gcc/reload1.c	(revision 150048)
--- gcc/reload1.c	(working copy)
*************** reload_as_needed (int live_known)
*** 4304,4334 ****
  			    continue;
  			  if (n == 1)
  			    {
! 			      n = validate_replace_rtx (reload_reg,
! 							gen_rtx_fmt_e (code,
! 								       mode,
! 								       reload_reg),
! 							p);
  
  			      /* We must also verify that the constraints
  				 are met after the replacement.  */
  			      extract_insn (p);
  			      if (n)
  				n = constrain_operands (1);
- 			      else
- 				break;
  
  			      /* If the constraints were not met, then
! 				 undo the replacement.  */
  			      if (!n)
! 				{
! 				  validate_replace_rtx (gen_rtx_fmt_e (code,
! 								       mode,
! 								       reload_reg),
! 							reload_reg, p);
! 				  break;
! 				}
! 
  			    }
  			  break;
  			}
--- 4304,4328 ----
  			    continue;
  			  if (n == 1)
  			    {
! 			      rtx replace_reg
! 				= gen_rtx_fmt_e (code, mode, reload_reg);
! 
! 			      validate_replace_rtx_group (reload_reg,
! 							  replace_reg, p);
! 			      n = verify_changes (0);
  
  			      /* We must also verify that the constraints
  				 are met after the replacement.  */
  			      extract_insn (p);
  			      if (n)
  				n = constrain_operands (1);
  
  			      /* If the constraints were not met, then
! 				 undo the replacement, else confirm it.  */
  			      if (!n)
! 				cancel_changes (0);
! 			      else
! 				confirm_change_group ();
  			    }
  			  break;
  			}


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com