[4.1 Committed] PR middle-end/26729: Fold vs. TREE_OVERFLOW

Roger Sayle roger@eyesopen.com
Sun May 14 09:47:00 GMT 2006


The following patch resolves PR middle-end/26729, which is a P1
invalid-code regression on the 4.0 and 4.1 branches.  I've spent
several days attempting to backport the various TREE_OVERFLOW
clean-ups that have fixed this problem on mainline, to discover
that this issue has been cured by the cumulative effects of a
large number of minor improvements.  I eventually gave up trying
to backport the five or six (at least) patches, including changes
to integer_zerop, convert_to_pointer, fold_binary, const_binop
and various places in the C/C++ front-end.  At least, I now have
a much better appreciation for how small incremental improvements
can combine to correct significant middle-end design issues.

Instead, I decided to investigate a more direct fix for the
wrong-code regression described in the PR, and was surprised at
how simple/safe a fix I found.  This seems like a much better
short-term workaround that a series of invasive clean-ups for
the 4.1.2 release.

The problem is that the current code is checking "!integer_zerop"
to determine whether an integer constant is not zero, which is
tripping over the odd semantics that overflowed constants are
treated like NaNs and always test false.  The quick fix is to
instead test "integer_nonzerop", which inverts the sense of the
comparison, so now TREE_CONSTANT_OVERFLOW does the right thing.


The following patch has been tested against the gcc-4_1-branch on
i686-pc-linux-gnu with a full "make bootstrap", all default languages
including Ada, and regression tested with a top-level "make -k check"
with no new failures.

Committed to the gcc-4_1-branch as revision 113755.  I'll commit
the equivalent patch to the 4.0 branch, once bootstrap and regression
testing complete there.  I'll also check the new test case into
mainline to prevent similar problems in future.


2006-05-13  Roger Sayle  <roger@eyesopen.com>

	PR middle-end/26729
	* fold-const.c (fold_truthop): Check integer_nonzerop instead of
	!integer_zerop to avoid problems with TREE_OVERFLOW.

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


Index: fold-const.c
===================================================================
*** fold-const.c	(revision 113695)
--- fold-const.c	(working copy)
*************** fold_truthop (enum tree_code code, tree
*** 4935,4944 ****
        l_const = fold_convert (lntype, l_const);
        l_const = unextend (l_const, ll_bitsize, ll_unsignedp, ll_and_mask);
        l_const = const_binop (LSHIFT_EXPR, l_const, size_int (xll_bitpos), 0);
!       if (! integer_zerop (const_binop (BIT_AND_EXPR, l_const,
! 					fold_build1 (BIT_NOT_EXPR,
! 						     lntype, ll_mask),
! 					0)))
  	{
  	  warning (0, "comparison is always %d", wanted_code == NE_EXPR);

--- 4935,4944 ----
        l_const = fold_convert (lntype, l_const);
        l_const = unextend (l_const, ll_bitsize, ll_unsignedp, ll_and_mask);
        l_const = const_binop (LSHIFT_EXPR, l_const, size_int (xll_bitpos), 0);
!       if (integer_nonzerop (const_binop (BIT_AND_EXPR, l_const,
! 					 fold_build1 (BIT_NOT_EXPR,
! 						      lntype, ll_mask),
! 					 0)))
  	{
  	  warning (0, "comparison is always %d", wanted_code == NE_EXPR);

*************** fold_truthop (enum tree_code code, tree
*** 4950,4959 ****
        r_const = fold_convert (lntype, r_const);
        r_const = unextend (r_const, rl_bitsize, rl_unsignedp, rl_and_mask);
        r_const = const_binop (LSHIFT_EXPR, r_const, size_int (xrl_bitpos), 0);
!       if (! integer_zerop (const_binop (BIT_AND_EXPR, r_const,
! 					fold_build1 (BIT_NOT_EXPR,
! 						     lntype, rl_mask),
! 					0)))
  	{
  	  warning (0, "comparison is always %d", wanted_code == NE_EXPR);

--- 4950,4959 ----
        r_const = fold_convert (lntype, r_const);
        r_const = unextend (r_const, rl_bitsize, rl_unsignedp, rl_and_mask);
        r_const = const_binop (LSHIFT_EXPR, r_const, size_int (xrl_bitpos), 0);
!       if (integer_nonzerop (const_binop (BIT_AND_EXPR, r_const,
! 					 fold_build1 (BIT_NOT_EXPR,
! 						      lntype, rl_mask),
! 					 0)))
  	{
  	  warning (0, "comparison is always %d", wanted_code == NE_EXPR);



/* { dg-do run } */
/* { dg-options "-O2" } */

void abort(void);

__attribute__((noinline))
int f (unsigned short word) {
  return (word & 0x1) && (((unsigned short) (word & 0x8000)) == 0x8000);
}

int main(void) {
  if (!f(0x8001))
    abort();
  return 0;
}


Roger
--



More information about the Gcc-patches mailing list