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]

compare_from_rtx problem



In certain circumstances compare_from_rtx may reverse the order of operands
in a comparison as well as swapping the code for the comparison.

This can happen (for example) when comparing register to a queued expression
(we want the queued expression to be the first operand, so we perform the
swap).

Unfortunately, the callers of compare_from_rtx do not check if the comparison
code was reversed.  This can cause them to generate the incorrect comparison -
particularly on ports where cmpXX insns just save the operands and depend on
the branch patterns to extract the operands and create a suitable branch.

This test (which I'll install momentarily) exposes the problem.  It's known
to fail on the PA, and I'd expect it to fail on the Sparc and various
other ports.

int
main()
{
  double x = 1.0;
  double y = 2.0;

  if ((y > x--) != 1)
    abort ();
  exit (0);
}


Luckily, fixing the compiler is pretty simple as compare_from_rtx is
only called from 3 sites.

The first is in optabs.c.    It luckily checks to see if the returned
comparison's code is any different from the original code.  If it is
different, then we merely lose the chance to generate a conditional
move -- ie, no incorrect code will be generated.  I did not change
this call site in any way.

In expmed.c we check to see if the comparison code changed, and if it did,
we abort.  Opps.  I've updated the code slightly to extract the updated
comparison code from the return value of compare_from_rtx.  All should be
well.

The last (and the one which matters for the testcase above) is in expr.c;
where it doesn't perform any sanity checks at all, it just blindly generates
the wrong code.  I've updated it to extract the comparison code from the
return value of compare_from_rtx.  And, gee whiz, the bug is fixed.

This has been bootstrapped and regression tested on ia32, PA32 and Sparc32.

	* expmed.c (emit_store_flag): Extract updated comparison code
	from the return value of compare_from_rtx.
	(do_store_flag): Similarly.

Index: expmed.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/expmed.c,v
retrieving revision 1.162
diff -c -3 -p -r1.162 expmed.c
*** expmed.c	2001/10/22 08:35:01	1.162
--- expmed.c	2001/12/17 20:24:08
*************** emit_store_flag (target, code, op0, op1,
*** 4387,4398 ****
  		: normalizep == -1 ? constm1_rtx
  		: const_true_rtx);
  
!       /* If the code of COMPARISON doesn't match CODE, something is
! 	 wrong; we can no longer be sure that we have the operation.  
! 	 We could handle this case, but it should not happen.  */
  
!       if (GET_CODE (comparison) != code)
! 	abort ();
  
        /* Get a reference to the target in the proper mode for this insn.  */
        compare_mode = insn_data[(int) icode].operand[0].mode;
--- 4387,4399 ----
  		: normalizep == -1 ? constm1_rtx
  		: const_true_rtx);
  
!       /* The code of COMPARISON may not match CODE if compare_from_rtx
! 	 decided to swap its operands and reverse the original code.
  
! 	 We know that compare_from_rtx returns either a CONST_INT or
! 	 a new comparison code, so it is safe to just extract the
! 	 code from COMPARISON.  */
!       code = GET_CODE (comparison);
  
        /* Get a reference to the target in the proper mode for this insn.  */
        compare_mode = insn_data[(int) icode].operand[0].mode;
Index: expr.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/expr.c,v
retrieving revision 1.453.22.3
diff -c -3 -p -r1.453.22.3 expr.c
*** expr.c	2001/12/12 22:51:41	1.453.22.3
--- expr.c	2001/12/17 20:24:19
*************** do_store_flag (exp, target, mode, only_c
*** 10768,10773 ****
--- 10768,10781 ----
  	     || (result != const0_rtx && invert))
  	    ? const0_rtx : const1_rtx);
  
+   /* The code of RESULT may not match CODE if compare_from_rtx
+      decided to swap its operands and reverse the original code.
+ 
+      We know that compare_from_rtx returns either a CONST_INT or
+      a new comparison code, so it is safe to just extract the
+      code from RESULT.  */
+   code = GET_CODE (result);
+ 
    label = gen_label_rtx ();
    if (bcc_gen_fctn[(int) code] == 0)
      abort ();



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