[PATCH] Fix incorrect handling of undobuf.other_insn in combine

["Ulrich Weigand" <weigand at i1 dot informatik dot uni-erlangen dot de>]

Hello,

I've seen a bug where combine would change some other insn without
properly validating that change.  The mechanism to avoid this
happening usually is the 'undobuf.other_insn' field; if this is
set, the insn specified there is validated or else the whole
combine attempt is cancelled.

However, there is one code path in simplify_set that can cause
a previously set undobuf.other_insn to be cleared out again,
which causes the bug in my test case.

This code path goes as follows:

  /* If we are setting CC0 or if the source is a COMPARE, look for the use of
     the comparison result and try to simplify it unless we already have used
     undobuf.other_insn.  */
  if ((GET_MODE_CLASS (mode) == MODE_CC
       || GET_CODE (src) == COMPARE
       || CC0_P (dest))
      && (cc_use = find_single_use (dest, subst_insn, &other_insn)) != 0
      && (undobuf.other_insn == 0 || other_insn == undobuf.other_insn)

Note that the 'if' can be entered if undobuf.other_insn was non-zero 
(because it already held the same insn as the one we're looking at 
right now) ...

      /* If the code changed, we have to build a new comparison in
         undobuf.other_insn.  */
      if (new_code != old_code)
        {
          unsigned HOST_WIDE_INT mask;

          SUBST (*cc_use, gen_rtx_fmt_ee (new_code, GET_MODE (*cc_use),
                                          dest, const0_rtx));

          /* If the only change we made was to change an EQ into an NE or
             vice versa, OP0 has only one bit that might be nonzero, and OP1
             is zero, check if changing the user of the condition code will
             produce a valid insn.  If it won't, we can keep the original code
             in that insn by surrounding our operation with an XOR.  */

          if (((old_code == NE && new_code == EQ)
               || (old_code == EQ && new_code == NE))
              && ! other_changed && op1 == const0_rtx
              && GET_MODE_BITSIZE (GET_MODE (op0)) <= HOST_BITS_PER_WIDE_INT
              && exact_log2 (mask = nonzero_bits (op0, GET_MODE (op0))) >= 0)
            {
              rtx pat = PATTERN (other_insn), note = 0;

              if ((recog_for_combine (&pat, other_insn, &note) < 0
                   && ! check_asm_operands (pat)))
                {
                  PUT_CODE (*cc_use, old_code);
                  other_insn = 0;

                  op0 = gen_binary (XOR, GET_MODE (op0), op0, GEN_INT (mask));
                }
            }

          other_changed = 1;
        }

Note also that there is a path through this that leaves 'other_insn' == 0 and
'other_changed' == 1 ...

      if (other_changed)
        undobuf.other_insn = other_insn;

... which causes the existing non-zero undobuf.other_insn to be zeroed out.

Obviously, this is not what was intended here; the point of that inner 'if'
was simply to prevent undobuf.other_insn from being set, not to actually clear
it if had already been set previously.

The following patch fixes this.

Bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux.
OK?

ChangeLog:

	* combine.c (simplify_set): Do not clear out undobuf.other_insn
	already set elsewhere.

Index: gcc/combine.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.388
diff -c -p -r1.388 combine.c
*** gcc/combine.c	26 Sep 2003 06:05:48 -0000	1.388
--- gcc/combine.c	28 Sep 2003 21:33:49 -0000
*************** simplify_set (rtx x)
*** 5056,5065 ****
--- 5056,5067 ----
  	 undobuf.other_insn.  */
        if (new_code != old_code)
  	{
+ 	  int other_changed_previously = other_changed;
  	  unsigned HOST_WIDE_INT mask;
  
  	  SUBST (*cc_use, gen_rtx_fmt_ee (new_code, GET_MODE (*cc_use),
  					  dest, const0_rtx));
+ 	  other_changed = 1;
  
  	  /* If the only change we made was to change an EQ into an NE or
  	     vice versa, OP0 has only one bit that might be nonzero, and OP1
*************** simplify_set (rtx x)
*** 5069,5075 ****
  
  	  if (((old_code == NE && new_code == EQ)
  	       || (old_code == EQ && new_code == NE))
! 	      && ! other_changed && op1 == const0_rtx
  	      && GET_MODE_BITSIZE (GET_MODE (op0)) <= HOST_BITS_PER_WIDE_INT
  	      && exact_log2 (mask = nonzero_bits (op0, GET_MODE (op0))) >= 0)
  	    {
--- 5071,5077 ----
  
  	  if (((old_code == NE && new_code == EQ)
  	       || (old_code == EQ && new_code == NE))
! 	      && ! other_changed_previously && op1 == const0_rtx
  	      && GET_MODE_BITSIZE (GET_MODE (op0)) <= HOST_BITS_PER_WIDE_INT
  	      && exact_log2 (mask = nonzero_bits (op0, GET_MODE (op0))) >= 0)
  	    {
*************** simplify_set (rtx x)
*** 5079,5091 ****
  		   && ! check_asm_operands (pat)))
  		{
  		  PUT_CODE (*cc_use, old_code);
! 		  other_insn = 0;
  
  		  op0 = gen_binary (XOR, GET_MODE (op0), op0, GEN_INT (mask));
  		}
  	    }
- 
- 	  other_changed = 1;
  	}
  
        if (other_changed)
--- 5081,5091 ----
  		   && ! check_asm_operands (pat)))
  		{
  		  PUT_CODE (*cc_use, old_code);
! 		  other_changed = 0;
  
  		  op0 = gen_binary (XOR, GET_MODE (op0), op0, GEN_INT (mask));
  		}
  	    }
  	}
  
        if (other_changed)
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de