Re: [PATCH] Fix ICE in noce_try_sign_mask, fallout from rtx_cost changes (PR target/37483)

[Adam Nemet <anemet at caviumnetworks dot com>]

Jakub Jelinek <jakub@redhat.com> writes:
> if_info->insn_b can be NULL (in the if (test) x = 0; => x = (test >> 31) & x;
> case) and t could come either from insn_a's or insn_b's block, but more
> importantly, we are considering evaluating t in if_info->test_bb, so IMHO we
> should just use the target bb's speed/size state, not the bb of the branch
> where it has been computed before (for insn_b == NULL that would need to
> be test_bb anyway).

I think this is reasonable.  I worked on a duplicate of this bug (37396: mips
bootstrap failure) and for the record this is what I was going to submit this
morning.  This patch does a bit more clean up on the code in question and uses
the BB of insn_a and insn_b for the cost computation.

At this point I bootstrapped and tested it with x86_64-linux-gnu and I also
bootstrapped it with mips-lixux-gnu.  I have mips-linux-gnu testresults but I
still need to look at it.  I just wanted to submit this for discussion since
Jakub's patch was submitted.

Adam

	PR target/37395
	PR target/37483
	* ifcvt.c (noce_try_sign_mask): Use the appropriate basic block of
	either INSN_A or INSN_B to compute the cost of evaluating T.
	Don't look up the block of INSN_B if INSN_B is null.

testsuite/
	* gcc.dg/torture/pr37395.c: New test.

Index: ifcvt.c
===================================================================
*** ifcvt.c	(revision 139918)
--- ifcvt.c	(working copy)
*************** noce_try_sign_mask (struct noce_if_info 
*** 1869,1875 ****
    rtx cond, t, m, c, seq;
    enum machine_mode mode;
    enum rtx_code code;
-   bool b_unconditional;
  
    cond = if_info->cond;
    code = GET_CODE (cond);
--- 1869,1874 ----
*************** noce_try_sign_mask (struct noce_if_info 
*** 1898,1913 ****
    if (GET_MODE (m) != mode)
      return FALSE;
  
!   /* This is only profitable if T is cheap, or T is unconditionally
!      executed/evaluated in the original insn sequence.  The latter
!      happens if INSN_B was taken from TEST_BB, or if there was no
!      INSN_B which can happen for e.g. conditional stores to memory.  */
!   b_unconditional = (if_info->insn_b == NULL_RTX
! 		     || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb);
!   if (rtx_cost (t, SET, optimize_bb_for_speed_p (BLOCK_FOR_INSN (if_info->insn_b)))
!       >= COSTS_N_INSNS (2)
!       && (!b_unconditional
!           || t != if_info->b))
      return FALSE;
  
    start_sequence ();
--- 1897,1921 ----
    if (GET_MODE (m) != mode)
      return FALSE;
  
!   /* This is only profitable if T is cheap.  */
!   if (if_info->b == t)
!     {
!       /* It's also profitable if the value T is evaluated/executed
! 	 unconditionally.  This is the case if INSN_B was taken from
! 	 TEST_BB or if there was no INSN_B which can happen for
! 	 e.g. conditional stores to memory.  */
!       if (!(if_info->insn_b == NULL_RTX
! 	    || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb
! 	    || (rtx_cost (t, SET,
! 			  optimize_bb_for_speed_p
! 			  (BLOCK_FOR_INSN (if_info->insn_b)))
! 		< COSTS_N_INSNS (2))))
! 	return FALSE;
!     }
!   else if (rtx_cost (t, SET,
! 		     optimize_bb_for_speed_p
! 		     (BLOCK_FOR_INSN (if_info->insn_a)))
! 	   >= COSTS_N_INSNS (2))
      return FALSE;
  
    start_sequence ();
Index: testsuite/gcc.dg/torture/pr37395.c
===================================================================
*** testsuite/gcc.dg/torture/pr37395.c	(revision 0)
--- testsuite/gcc.dg/torture/pr37395.c	(revision 0)
***************
*** 0 ****
--- 1,11 ----
+ /* { dg-do compile } */
+ 
+ int
+ f (int j)
+ {
+   int i;
+   asm volatile ("" : "=r"(i));
+   if (i >= 0)
+     j = 0;
+   return j;
+ }