Re: [RFA] A patch for PR 25115 (take 2)

[Roger Sayle <roger at eyesopen dot com>]

On Sat, 17 Dec 2005, Paolo Bonzini wrote:
> > Therefore I think the correct approach is to assume that if the
> > pattern is a SET, then that is the "set" we're interested in (as we
> > do currently).  For, the PARALLEL case, I propose looping over the
> > elements of the parallel looking for a single set... If there's only one,
> > this must be the one we're interested in.  Otherwise, we should loop
> > across the parallel using the existing expr_equiv_p logic.  This final
> > fallback should never be required in the case of values taken from notes.
>
> Like this?

Not quite.  I think its better to continue to search for the set ourselves
(matching the logic in hash_scan_set) than use single_set.  The reason
being the mismatch between the handling of unused results in parallel
instructions between hash_scan_insn and single_set.  In GCSE, the compiler
may decide to make use of a result generated by a parallel that was
previously unused.  If we call single_set, we may end up identifying the
the wrong (live) set.  In fact, if memory serves, this is the reason why
we stopped using single_set in these GCSE functions, and the motivation
behind the "This block matches the logic in hash_scan_set" comment.

Sorry if I wasn't clear in the description above, but I was thinking
something like the following...  If there's only one set we always
use that, if there are more than one, we use the (first) one that's
equivalent to the expression that we're looking for.



Index: gcse.c
===================================================================
*** gcse.c	(revision 108622)
--- gcse.c	(working copy)
*************** pre_insert_copy_insn (struct expr *expr,
*** 4218,4223 ****
--- 4218,4224 ----
    int regno = REGNO (reg);
    int indx = expr->bitmap_index;
    rtx pat = PATTERN (insn);
+   bool multiple_sets;
    rtx set, new_insn;
    rtx old_reg;
    int i;
*************** pre_insert_copy_insn (struct expr *expr,
*** 4233,4246 ****
        /* Search through the parallel looking for the set whose
  	 source was the expression that we're interested in.  */
        set = NULL_RTX;
        for (i = 0; i < XVECLEN (pat, 0); i++)
  	{
  	  rtx x = XVECEXP (pat, 0, i);
! 	  if (GET_CODE (x) == SET
! 	      && expr_equiv_p (SET_SRC (x), expr->expr))
  	    {
! 	      set = x;
! 	      break;
  	    }
  	}
        break;
--- 4234,4267 ----
        /* Search through the parallel looking for the set whose
  	 source was the expression that we're interested in.  */
        set = NULL_RTX;
+       multiple_sets = false;
        for (i = 0; i < XVECLEN (pat, 0); i++)
  	{
  	  rtx x = XVECEXP (pat, 0, i);
! 	  if (GET_CODE (x) == SET)
  	    {
! 	      if (multiple_sets)
! 		{
! 		  if (expr_equiv_p (SET_SRC (x), expr->expr))
! 		    {
! 		      set = x;
! 		      break;
! 		    }
! 		}
! 	      else if (set)
! 		{
! 		  if (expr_equiv_p (SET_SRC (set), expr->expr))
! 		    break;
! 		  if (expr_equiv_p (SET_SRC (x), expr->expr))
! 		    {
! 		      set = x;
! 		      break;
! 		    }
! 		  multiple_sets = true;
! 		  set = NULL_RTX;
! 		}
! 	      else
! 		set = x;
  	    }
  	}
        break;
*************** pre_insert_copy_insn (struct expr *expr,
*** 4249,4254 ****
--- 4270,4277 ----
        gcc_unreachable ();
      }

+   gcc_assert (set != NULL_RTX);
+
    if (REG_P (SET_DEST (set)))
      {
        old_reg = SET_DEST (set);


Roger
--