[PATCH] PR 15069: types.signed_type lang hook misunderstanding

[Roger Sayle <roger at eyesopen dot com>]

The following patch is one solution to PR middle-end/15069 which is
an "incorrect code" regression on mainline and 3.4.  The miscompilation
concerns the constant folding transformation "(A & C) != 0" ->
"(signed)A < 0" when C is the most significant bit of A.

The issue is that in the PR the type of A is an enumeration type,
represented by a three-bit wide unsigned integer.  Fold then calls
lang_hooks.types.signed_type on this type expecting to get a three-bit
wide signed type.  Unfortunately, the C++ front-end actually returns
a signed char.  This miscommunication means that the tree "(signed)A"
doesn't actually preserve the sign-bit, as the unsigned enumerated type
is zero-extended to fill the signed char.  Doh!


Although this may be a C++ front-end bug, it is possible to fix this
particular regression by guarding this constant folding transformation
with a test to check that the TYPE_PRECISION of the type returned from
lang_hooks.types.signed_type has the same precision as the original.

Unfortunately, I expect that there may be numerous places in the
middle-end that assume that signed_type preserves precision, so if
this is the correct fix, I'll need to check all other uses.

I'm also not sure this transformation is always a win unless the
type is a full machine mode, i.e. GET_MODE_BITSIZE (TYPE_MODE (type))
== TYPE_PRECISION (mode), so it might make sense to also add this
check to this transformation.

The alternate approach of tweaking the g++ front-end to return a
signed enumerated type would both resolve this problem, and permit
a number of these optimizations that would otherwise have to wait
until RTL. i.e. tree-ssa may be unable to perform transformations
as the tree type system is not expressive enough to handle changing
signedness of all integral types.


The following patch has been tested on i686-pc-linux-gnu with a full
"make bootstrap", all languages except treelang, and regression tested
with a top-level "make -k check" with no new failures.

Is this the correct fix for mainline/3.4?



2004-04-24  Roger Sayle  <roger@eyesopen.com>

	PR middle-end/15069
	* fold-const.c (fold_single_bit_test): Check that the type returned
	from lang_hooks.types.signed_type has the same precision as its
	operand, otherwise converting between types doesn't preserve sign.

	* g++.dg/opt/fold3.C: New test case.


Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.369
diff -c -3 -p -r1.369 fold-const.c
*** fold-const.c	20 Apr 2004 02:56:44 -0000	1.369
--- fold-const.c	24 Apr 2004 15:02:41 -0000
*************** fold_single_bit_test (enum tree_code cod
*** 5255,5269 ****
        if (arg00 != NULL_TREE)
  	{
  	  tree stype = lang_hooks.types.signed_type (TREE_TYPE (arg00));
! 	  return fold (build (code == EQ_EXPR ? GE_EXPR : LT_EXPR, result_type,
! 			      fold_convert (stype, arg00),
! 			      fold_convert (stype, integer_zero_node)));
  	}

-       /* At this point, we know that arg0 is not testing the sign bit.  */
-       if (TYPE_PRECISION (type) - 1 == bitnum)
- 	abort ();
-
        /* Otherwise we have (A & C) != 0 where C is a single bit,
  	 convert that into ((A >> C2) & 1).  Where C2 = log2(C).
  	 Similarly for (A & C) == 0.  */
--- 5255,5268 ----
        if (arg00 != NULL_TREE)
  	{
  	  tree stype = lang_hooks.types.signed_type (TREE_TYPE (arg00));
! 	  /* Check that conversion to stype preserves the sign bit.  */
! 	  if (TYPE_PRECISION (stype) == TYPE_PRECISION (TREE_TYPE (arg00))
! 	      || ! TYPE_UNSIGNED (TREE_TYPE (arg00)))
! 	    return fold (build (code == EQ_EXPR ? GE_EXPR : LT_EXPR,
! 				result_type, fold_convert (stype, arg00),
! 				fold_convert (stype, integer_zero_node)));
  	}

        /* Otherwise we have (A & C) != 0 where C is a single bit,
  	 convert that into ((A >> C2) & 1).  Where C2 = log2(C).
  	 Similarly for (A & C) == 0.  */


// PR middle-end/15069
// { dg-do run }
// { dg-options "-O2" }

extern "C" void abort (void);

typedef enum {
  FOUR = 4,
  FIVE = 5
} direction_t;

int main ()
{
  direction_t four = FOUR;
  int flags = (four & 4L) ? (32L | 128L) : 0;
  flags &= 32L;

  if (flags == 0)
    abort ();
}



Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833