Re: Speedup CSE by 5%

[Jeffrey A Law <law at redhat dot com>]

On Thu, 2005-01-27 at 07:24 -0500, Jakub Jelinek wrote:
> On Thu, Jan 27, 2005 at 01:12:43PM +0100, Bernd Schmidt wrote:
> > >>I understand all that.  However, if you look at the first call to
> > >>validate_change (inside an ASM_OPERANDS case) you'll see a case where
> > >>I think we can modify x without copying it first.
> > >
> > >
> > >I think the proposed patch is actually safe.  Consider
> > [...]
> > >Just to make things clear, we might want to do something like this:
> > >
> > >    case ASM_OPERANDS:
> > >      if (insn)
> > >	{
> > >	  for (i = ASM_OPERANDS_INPUT_LENGTH (x) - 1; i >= 0; i--)
> > >	    validate_change (insn, &ASM_OPERANDS_INPUT (x, i),
> > >			     fold_rtx (ASM_OPERANDS_INPUT (x, i), insn), 0);
> > >	}
> > >      break;
> > 
> > Agreed.
> > 
> > There are two possible reasons for the copy_rtx I can think of: either 
> > fold_rtx destructively modifies its input (which I shouldn't happen from 
> > what I can see), or we have RTL sharing issues.  In the latter case, we 
> > can postpone the copy until after we found that fold_rtx has made a change.
> > 
> > I did a bit of archaeology; the call to copy_rtx was introduced as part 
> > of a larger change by kenner in
> > 
> > http://savannah.gnu.org/cgi-bin/viewcvs/gcc/old-gcc/cse.c.diff?r1=1.165&r2=1.166
> 
> BTW, the copy_rtx call as argument to fold_rtx in find_best_addr isn't the only
> one in cse.c, there is also
>     case NOT:
>     case NEG:
>       /* If we have (NOT Y), see if Y is known to be (NOT Z).
>          If so, (NOT Y) simplifies to Z.  Similarly for NEG.  */
>       new = lookup_as_function (XEXP (x, 0), code);
>       if (new)
>         return fold_rtx (copy_rtx (XEXP (new, 0)), insn);
> and
>         case MINUS:
>           /* If we have (MINUS Y C), see if Y is known to be (PLUS Z C2).
>              If so, produce (PLUS Z C2-C).  */
>           if (const_arg1 != 0 && GET_CODE (const_arg1) == CONST_INT)
>             {
>               rtx y = lookup_as_function (XEXP (x, 0), PLUS);
>               if (y && GET_CODE (XEXP (y, 1)) == CONST_INT)
>                 return fold_rtx (plus_constant (copy_rtx (y),
>                                                 -INTVAL (const_arg1)),
>                                  NULL_RTX);
>             }
> At least the latter is similar to the one in find_best_addr, in that
> it is called with NULL_RTX argument, therefore fold_rtx should be making
> a copy instead of modifying in place according to fold_rtx documentation.
The NOT/NEG case could probably be handled with something like this:

return fold_rtx (insn ? copy_rtx (XEXP (new, 0)) : XEXP (new, 0), insn);

The MINUS case assumes that plus_constant doesn't modify RTL in-place,
which AFAIK is a safe assumption.

I'll give those two tweaks a spin and see what happens...  Even if we
can't measure a speedup, we save memory when we can avoid the copy.

jeff