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 15825: emit_move_insn problems in ifcvt.c (take 2)


On Thu, 24 Jun 2004, Richard Henderson wrote:
> On Sat, Jun 19, 2004 at 02:07:10PM -0600, Roger Sayle wrote:
> > I have a patch for an almost identical problem, PR 15825, awaiting
> > review at http://gcc.gnu.org/ml/gcc-patches/2004-06/msg00338.html
> > If you prefer your solution of rejecting the sequence, let me know
> > and I'll test with your identical hunk added to noce_try_sign_mask.
>
> I think I do prefer my method, simply because it's safer.
>
> > It might even be worthwhile factoring this code into its own function
> > in ifcvt.c, or perhaps just reorganize unshare_ifcvt_sequence.
>
> Agreed.


The following patch implements Richard's recommendations to factor the
new insn recognition checks into a unshare_ifcvt_sequence function, and
reuse it during if-conversion.  This patch fixes PR middle-end/15825.

As a follow-up patch, I'll propose inlining seq_contains_jump
directly into end_ifcvt_sequence, it's only remaining caller, to
minimize any additional overhead with using this approach.
Pre-approval?


The following patch has been tested on i686-pc-linux-gnu with a full
"make bootstrap", all default languages, and regression tested with a
top-level "make -k check" with no new failures.  I've also confirmed
that on powerpc-apple-darwin7.2.0 that it resolves the ICE in the
attached testcase.  Unfortunately, the libiberty changes to mudflap
currently prevent full bootstrap/reg-tests on darwin.

Ok for mainline?


2004-06-25  Roger Sayle  <roger@eyesopen.com>

	PR middle-end/15825
	* ifcvt.c (unshare_ifcvt_sequence): Rename to end_ifcvt_sequence.
	Use get_isns and end_sequence instead of accepting a seq argument.
	Scan the instruction sequence for unrecognizable or jump insns.
	(noce_try_move, noce_try_store_flag, noce_try_store_flag_constants,
	noce_try_addcc, noce_try_store_flag_mask, noce_try_cmove,
	noce_try_cmove_arith, noce_try_minmax, noce_try_abs,
	noce_try_sign_mask): Use end_ifcvt_sequence to factor common code.

	* gcc.dg/pr15825-1.c: New test case.


Index: ifcvt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ifcvt.c,v
retrieving revision 1.147
diff -c -3 -p -r1.147 ifcvt.c
*** ifcvt.c	19 Jun 2004 19:13:03 -0000	1.147
--- ifcvt.c	24 Jun 2004 23:23:00 -0000
*************** noce_emit_move_insn (rtx x, rtx y)
*** 699,712 ****
  		   GET_MODE_BITSIZE (inmode));
  }

! /* Unshare sequence SEQ produced by if conversion.  We care to mark
!    all arguments that may be shared with outer instruction stream.  */
! static void
! unshare_ifcvt_sequence (struct noce_if_info *if_info, rtx seq)
  {
    set_used_flags (if_info->x);
    set_used_flags (if_info->cond);
    unshare_all_rtl_in_chain (seq);
  }

  /* Convert "if (a != b) x = a; else x = b" into "x = a" and
--- 699,730 ----
  		   GET_MODE_BITSIZE (inmode));
  }

! /* Return sequence of instructions generated by if conversion.  This
!    function calls end_sequence() to end the current stream, ensures
!    that are instructions are unshared, recognizable non-jump insns.
!    On failure, this function returns a NULL_RTX.  */
!
! static rtx
! end_ifcvt_sequence (struct noce_if_info *if_info)
  {
+   rtx y;
+   rtx seq = get_insns ();
+
    set_used_flags (if_info->x);
    set_used_flags (if_info->cond);
    unshare_all_rtl_in_chain (seq);
+   end_sequence ();
+
+   if (seq_contains_jump (seq))
+     return NULL_RTX;
+
+   /* Make sure that all of the instructions emitted are recognizable.
+      As an excersise for the reader, build a general mechanism that
+      allows proper placement of required clobbers.  */
+   for (y = seq; y ; y = NEXT_INSN (y))
+     if (recog_memoized (y) == -1)
+       return NULL_RTX;
+   return seq;
  }

  /* Convert "if (a != b) x = a; else x = b" into "x = a" and
*************** noce_try_move (struct noce_if_info *if_i
*** 742,758 ****
  	{
  	  start_sequence ();
  	  noce_emit_move_insn (if_info->x, y);
! 	  seq = get_insns ();
! 	  unshare_ifcvt_sequence (if_info, seq);
! 	  end_sequence ();
!
! 	  /* Make sure that all of the instructions emitted are
! 	     recognizable.  As an excersise for the reader, build
! 	     a general mechanism that allows proper placement of
! 	     required clobbers.  */
! 	  for (y = seq; y ; y = NEXT_INSN (y))
! 	    if (recog_memoized (y) == -1)
! 	      return FALSE;

  	  emit_insn_before_setloc (seq, if_info->jump,
  				   INSN_LOCATOR (if_info->insn_a));
--- 760,768 ----
  	{
  	  start_sequence ();
  	  noce_emit_move_insn (if_info->x, y);
! 	  seq = end_ifcvt_sequence (if_info);
! 	  if (!seq)
! 	    return FALSE;

  	  emit_insn_before_setloc (seq, if_info->jump,
  				   INSN_LOCATOR (if_info->insn_a));
*************** noce_try_store_flag (struct noce_if_info
*** 795,805 ****
        if (target != if_info->x)
  	noce_emit_move_insn (if_info->x, target);

!       seq = get_insns ();
!       unshare_ifcvt_sequence (if_info, seq);
!       end_sequence ();
!       emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));

        return TRUE;
      }
    else
--- 805,816 ----
        if (target != if_info->x)
  	noce_emit_move_insn (if_info->x, target);

!       seq = end_ifcvt_sequence (if_info);
!       if (! seq)
! 	return FALSE;

+       emit_insn_before_setloc (seq, if_info->jump,
+ 			       INSN_LOCATOR (if_info->insn_a));
        return TRUE;
      }
    else
*************** noce_try_store_flag_constants (struct no
*** 926,940 ****
        if (target != if_info->x)
  	noce_emit_move_insn (if_info->x, target);

!       seq = get_insns ();
!       unshare_ifcvt_sequence (if_info, seq);
!       end_sequence ();
!
!       if (seq_contains_jump (seq))
  	return FALSE;

!       emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
!
        return TRUE;
      }

--- 937,948 ----
        if (target != if_info->x)
  	noce_emit_move_insn (if_info->x, target);

!       seq = end_ifcvt_sequence (if_info);
!       if (!seq)
  	return FALSE;

!       emit_insn_before_setloc (seq, if_info->jump,
! 			       INSN_LOCATOR (if_info->insn_a));
        return TRUE;
      }

*************** noce_try_addcc (struct noce_if_info *if_
*** 978,988 ****
  	      if (target != if_info->x)
  		noce_emit_move_insn (if_info->x, target);

! 	      seq = get_insns ();
! 	      unshare_ifcvt_sequence (if_info, seq);
! 	      end_sequence ();
  	      emit_insn_before_setloc (seq, if_info->jump,
! 				      INSN_LOCATOR (if_info->insn_a));
  	      return TRUE;
  	    }
  	  end_sequence ();
--- 986,997 ----
  	      if (target != if_info->x)
  		noce_emit_move_insn (if_info->x, target);

! 	      seq = end_ifcvt_sequence (if_info);
! 	      if (!seq)
! 		return FALSE;
!
  	      emit_insn_before_setloc (seq, if_info->jump,
! 				       INSN_LOCATOR (if_info->insn_a));
  	      return TRUE;
  	    }
  	  end_sequence ();
*************** noce_try_addcc (struct noce_if_info *if_
*** 1017,1032 ****
  	      if (target != if_info->x)
  		noce_emit_move_insn (if_info->x, target);

! 	      seq = get_insns ();
! 	      unshare_ifcvt_sequence (if_info, seq);
! 	      end_sequence ();
!
! 	      if (seq_contains_jump (seq))
  		return FALSE;

  	      emit_insn_before_setloc (seq, if_info->jump,
! 				      INSN_LOCATOR (if_info->insn_a));
!
  	      return TRUE;
  	    }
  	  end_sequence ();
--- 1026,1037 ----
  	      if (target != if_info->x)
  		noce_emit_move_insn (if_info->x, target);

! 	      seq = end_ifcvt_sequence (if_info);
! 	      if (!seq)
  		return FALSE;

  	      emit_insn_before_setloc (seq, if_info->jump,
! 				       INSN_LOCATOR (if_info->insn_a));
  	      return TRUE;
  	    }
  	  end_sequence ();
*************** noce_try_store_flag_mask (struct noce_if
*** 1071,1086 ****
  	  if (target != if_info->x)
  	    noce_emit_move_insn (if_info->x, target);

! 	  seq = get_insns ();
! 	  unshare_ifcvt_sequence (if_info, seq);
! 	  end_sequence ();
!
! 	  if (seq_contains_jump (seq))
  	    return FALSE;

  	  emit_insn_before_setloc (seq, if_info->jump,
! 				  INSN_LOCATOR (if_info->insn_a));
!
  	  return TRUE;
  	}

--- 1076,1087 ----
  	  if (target != if_info->x)
  	    noce_emit_move_insn (if_info->x, target);

! 	  seq = end_ifcvt_sequence (if_info);
! 	  if (!seq)
  	    return FALSE;

  	  emit_insn_before_setloc (seq, if_info->jump,
! 				   INSN_LOCATOR (if_info->insn_a));
  	  return TRUE;
  	}

*************** noce_try_cmove (struct noce_if_info *if_
*** 1169,1179 ****
  	  if (target != if_info->x)
  	    noce_emit_move_insn (if_info->x, target);

! 	  seq = get_insns ();
! 	  unshare_ifcvt_sequence (if_info, seq);
! 	  end_sequence ();
  	  emit_insn_before_setloc (seq, if_info->jump,
! 				  INSN_LOCATOR (if_info->insn_a));
  	  return TRUE;
  	}
        else
--- 1170,1181 ----
  	  if (target != if_info->x)
  	    noce_emit_move_insn (if_info->x, target);

! 	  seq = end_ifcvt_sequence (if_info);
! 	  if (!seq)
! 	    return FALSE;
!
  	  emit_insn_before_setloc (seq, if_info->jump,
! 				   INSN_LOCATOR (if_info->insn_a));
  	  return TRUE;
  	}
        else
*************** noce_try_cmove_arith (struct noce_if_inf
*** 1334,1342 ****
    else if (target != x)
      noce_emit_move_insn (x, target);

!   tmp = get_insns ();
!   unshare_ifcvt_sequence (if_info, tmp);
!   end_sequence ();
    emit_insn_before_setloc (tmp, if_info->jump, INSN_LOCATOR (if_info->insn_a));
    return TRUE;

--- 1336,1345 ----
    else if (target != x)
      noce_emit_move_insn (x, target);

!   tmp = end_ifcvt_sequence (if_info);
!   if (!tmp)
!     return FALSE;
!
    emit_insn_before_setloc (tmp, if_info->jump, INSN_LOCATOR (if_info->insn_a));
    return TRUE;

*************** noce_try_minmax (struct noce_if_info *if
*** 1580,1590 ****
    if (target != if_info->x)
      noce_emit_move_insn (if_info->x, target);

!   seq = get_insns ();
!   unshare_ifcvt_sequence (if_info, seq);
!   end_sequence ();
!
!   if (seq_contains_jump (seq))
      return FALSE;

    emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
--- 1583,1590 ----
    if (target != if_info->x)
      noce_emit_move_insn (if_info->x, target);

!   seq = end_ifcvt_sequence (if_info);
!   if (!seq)
      return FALSE;

    emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
*************** noce_try_abs (struct noce_if_info *if_in
*** 1698,1708 ****
    if (target != if_info->x)
      noce_emit_move_insn (if_info->x, target);

!   seq = get_insns ();
!   unshare_ifcvt_sequence (if_info, seq);
!   end_sequence ();
!
!   if (seq_contains_jump (seq))
      return FALSE;

    emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
--- 1698,1705 ----
    if (target != if_info->x)
      noce_emit_move_insn (if_info->x, target);

!   seq = end_ifcvt_sequence (if_info);
!   if (!seq)
      return FALSE;

    emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
*************** noce_try_sign_mask (struct noce_if_info
*** 1768,1778 ****
      }

    noce_emit_move_insn (if_info->x, t);
!   seq = get_insns ();
!   unshare_ifcvt_sequence (if_info, seq);
!   end_sequence ();
!   emit_insn_before_setloc (seq, if_info->jump,
! 			   INSN_LOCATOR (if_info->insn_a));
    return TRUE;
  }

--- 1765,1776 ----
      }

    noce_emit_move_insn (if_info->x, t);
!
!   seq = end_ifcvt_sequence (if_info);
!   if (!seq)
!     return FALSE;
!
!   emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
    return TRUE;
  }


/* PR middle-end/15825 */
/* { dg-do compile } */
/* { dg-options "-O2" } */

unsigned int foo(long long high, int unsigned_p)
{
  int i;
  if (high < 0)
    if (!unsigned_p)
    {
      i = 1;
      goto t;
    }
  i = 0;
t:
  return i;
}


Roger
--


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