PATCH: PR middle-end/33181: [4.3 Regression] Revision 127766 generates bad cmov

Uros Bizjak ubizjak@gmail.com
Sun Aug 26 16:36:00 GMT 2007


Hello!

> So, previous non-note insn from (insn 33) is (insn 28)(!!) in a totally
> unrelated BB, leading to all sort of strange things.
> ---
>
> This patch checks if the previous non-note insn is in the same basic
> block. I am testing it on Linux/x86-64 now. OK to install if pass?
>
>   
As mentioned by Richi in Comment #7, perhaps we should audit all uses of 
prev_nonnote_insn() in ifcfg.c? At least we need to check if BB of 
previous insn match BB of condition.
> --- gcc/ifcvt.c.cmov	2007-08-24 06:02:57.000000000 -0700
> +++ gcc/ifcvt.c	2007-08-26 07:35:50.000000000 -0700
> @@ -2198,6 +2198,7 @@ noce_process_if_block (struct noce_if_in
>  	 COND_EARLIEST to JUMP.  Make sure the relevant data is still
>  	 intact.  */
>        if (! insn_b
> +	  || BLOCK_NUM (insn_b) != BLOCK_NUM (if_info->cond_earliest)
>  	  || !NONJUMP_INSN_P (insn_b)
>  	  || (set_b = single_set (insn_b)) == NULL_RTX
>  	  || ! rtx_equal_p (x, SET_DEST (set_b))

Perhaps we can be a bit smarter here. Currently, only one (real) insn 
above the condition is blindly checked. However - is there perhaps a 
dataflow helper function that would find an insn with the store to the 
same location before the condition in the same BB? Something to 
substitute following mess:

  else
    {
      insn_b = prev_nonnote_insn (if_info->cond_earliest);
      /* We're going to be moving the evaluation of B down from above
     COND_EARLIEST to JUMP.  Make sure the relevant data is still
     intact.  */
      if (! insn_b
      || !NONJUMP_INSN_P (insn_b)
      || (set_b = single_set (insn_b)) == NULL_RTX
      || ! rtx_equal_p (x, SET_DEST (set_b))
      || reg_overlap_mentioned_p (x, SET_SRC (set_b))
      || modified_between_p (SET_SRC (set_b),
                 PREV_INSN (if_info->cond_earliest), jump)
      /* Likewise with X.  In particular this can happen when
         noce_get_condition looks farther back in the instruction
         stream than one might expect.  */
      || reg_overlap_mentioned_p (x, cond)
      || reg_overlap_mentioned_p (x, a)
      || modified_between_p (x, PREV_INSN (if_info->cond_earliest), jump))
    insn_b = set_b = NULL_RTX;
    }

Uros.



More information about the Gcc-patches mailing list