[PATCH] Optimize ((c>=1)&&(c<=127)) into ((signed char)c>0)

Roger Sayle roger@eyesopen.com
Wed May 8 19:38:00 GMT 2002


My recent patch to fix PR3995 uncovered an obscure optimization that
is not currently performed by the compiler.

The following code is taken from PR opt/3995.

void foo(signed char c) {
  if (c && !(c & 0x80)) {
    a();
  } else {
    b();
  }
}

With the recent patch mainline generates the following efficient
code (modulo stack tweaking) on IA-32 using "-O2 -fomit-frame-pointer".

_foo:	subl	$12, %esp
	cmpb	$0, 16(%esp)
	jle	L2
	addl	$12, %esp
	jmp	_a
L2:	addl	$12, %esp
	jmp	_b

Following Richard Henderson's suggestion, the transformation also
applies to unsigned types, so I'd expected changing "signed" to
"unsigned" in the above example to produce identical code.  Of
course I wouldn't be writing this if nothing strange happened...

_foo:	subl	$12, %esp
	movzbl	16(%esp), %eax
	decb	%al
	cmpb	$126, %al
	ja	L2
	addl	$12, %esp
	jmp	_a
L2:	addl	$12, %esp
	jmp	_b

After further investigation it transpires that GCC manages to convert
the original condition into "((c>=1) && (c<=127))" and from there
transforms the test into "(unsigned char)(c-1) <= 126" resulting in
the above sequence.


The patch below recovers the expected efficient code by teaching
fold that "(c>=1) && (c<=127)" can be efficiently tested with
the expression "(signed char)c > 0".  And the equivalent forms
for shorts, ints, longs, etc...

This patch has been tested on i686-pc-linux-gnu, i686-pc-cygwin
and alphaev67-dec-osf5.1 with "make bootstrap" and "make -k check"
with no new regressions (all languages except Ada).  I also include
a nominal test case that checks that the new code doesn't ICE and
the run-time results are as expected.

One comment on the changes.  I've replaced the if-else-ifs with
a sequence of ifs, as adding my new condition broke the original
aesthetic symmetry.  Given each if-block contained a return, I
thought using if's rather than else-if's clarifies the new control
flow.  I'm happy to change it back if anyone disagrees.

[Out of curiosity I checked and this optimization is so rare that
it never occurs in an entire bootstrap of the compiler, so I'm not
expecting a huge improvement in SPECint2000 :>]


Ok for mainline?


2002-05-08  Roger Sayle  <roger@eyesopen.com>

	* fold-const.c (build_range_check): Optimize (c>=1) && (c<=127)
	into the equivalent (signed char)c > 0.

	* gcc.c-torture/execute/20020508-1.c: New test case.


Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.200
diff -c -3 -p -r1.200 fold-const.c
*** fold-const.c	6 May 2002 22:59:37 -0000	1.200
--- fold-const.c	8 May 2002 19:54:08 -0000
*************** build_range_check (type, exp, in_p, low,
*** 3091,3131 ****
       tree low, high;
  {
    tree etype = TREE_TYPE (exp);
!   tree utype, value;

    if (! in_p
        && (0 != (value = build_range_check (type, exp, 1, low, high))))
      return invert_truthvalue (value);

!   else if (low == 0 && high == 0)
      return convert (type, integer_one_node);

!   else if (low == 0)
      return fold (build (LE_EXPR, type, exp, high));

!   else if (high == 0)
      return fold (build (GE_EXPR, type, exp, low));

!   else if (operand_equal_p (low, high, 0))
      return fold (build (EQ_EXPR, type, exp, low));

!   else if (TREE_UNSIGNED (etype) && integer_zerop (low))
!     return build_range_check (type, exp, 1, 0, high);
!
!   else if (integer_zerop (low))
      {
        utype = (*lang_hooks.types.unsigned_type) (etype);
        return build_range_check (type, convert (utype, exp), 1, 0,
  				convert (utype, high));
      }

!   else if (0 != (value = const_binop (MINUS_EXPR, high, low, 0))
! 	   && ! TREE_OVERFLOW (value))
      return build_range_check (type,
  			      fold (build (MINUS_EXPR, etype, exp, low)),
  			      1, convert (etype, integer_zero_node), value);
!   else
!     return 0;
  }

  /* Given two ranges, see if we can merge them into one.  Return 1 if we
--- 3091,3161 ----
       tree low, high;
  {
    tree etype = TREE_TYPE (exp);
!   tree utype, stype, value;

    if (! in_p
        && (0 != (value = build_range_check (type, exp, 1, low, high))))
      return invert_truthvalue (value);

!   if (low == 0 && high == 0)
      return convert (type, integer_one_node);

!   if (low == 0)
      return fold (build (LE_EXPR, type, exp, high));

!   if (high == 0)
      return fold (build (GE_EXPR, type, exp, low));

!   if (operand_equal_p (low, high, 0))
      return fold (build (EQ_EXPR, type, exp, low));

!   if (integer_zerop (low))
      {
+       if (TREE_UNSIGNED (etype))
+         return build_range_check (type, exp, 1, 0, high);
+
        utype = (*lang_hooks.types.unsigned_type) (etype);
        return build_range_check (type, convert (utype, exp), 1, 0,
  				convert (utype, high));
      }

!   /* Optimize (c>=1) && (c<=127) into (signed char)c > 0.  */
!   if (integer_onep (low) && TREE_CODE (high) == INTEGER_CST)
!     {
!       unsigned HOST_WIDE_INT lo;
!       HOST_WIDE_INT hi;
!       int prec;
!
!       prec = TYPE_PRECISION (etype);
!       if (prec <= HOST_BITS_PER_WIDE_INT)
!         {
!           hi = 0;
!           lo = ((unsigned HOST_WIDE_INT) 1 << (prec - 1)) - 1;
!         }
!       else
!         {
!           hi = ((HOST_WIDE_INT) 1 << (prec - HOST_BITS_PER_WIDE_INT - 1)) - 1;
!           lo = (unsigned HOST_WIDE_INT) -1;
!         }
!
!       if (TREE_INT_CST_HIGH (high) == hi && TREE_INT_CST_LOW (high) == lo)
!         {
!           if (! TREE_UNSIGNED (etype))
!             return fold (build (GT_EXPR, type, exp,
!                                 convert (etype, integer_zero_node)));
!           stype = (*lang_hooks.types.signed_type) (etype);
!           return fold (build (GT_EXPR, type, convert (stype, exp),
!                               convert (stype, integer_zero_node)));
!         }
!     }
!
!   if (0 != (value = const_binop (MINUS_EXPR, high, low, 0))
!       && ! TREE_OVERFLOW (value))
      return build_range_check (type,
  			      fold (build (MINUS_EXPR, etype, exp, low)),
  			      1, convert (etype, integer_zero_node), value);
!
!   return 0;
  }

  /* Given two ranges, see if we can merge them into one.  Return 1 if we
*** /dev/null	Thu Aug 30 14:30:55 2001
--- gcc.c-torture/execute/20020508-1.c	Wed May  8 13:56:46 2002
***************
*** 0 ****
--- 1,33 ----
+ /* Copyright (C) 2002  Free Software Foundation.
+
+    Test that optimizing ((c>=1) && (c<=127)) into (signed char)c < 0
+    doesn't cause any problems for the compiler and behaves correctly.
+
+    Written by Roger Sayle, 8th May 2002.  */
+
+ extern void abort (void);
+
+ void
+ test (unsigned char c, int ok)
+ {
+   if ((c>=1) && (c<=127))
+     {
+       if (!ok) abort ();
+     }
+   else
+     if (ok) abort ();
+ }
+
+ int
+ main ()
+ {
+   test (  0, 0);
+   test (  1, 1);
+   test ( 64, 1);
+   test (127, 1);
+   test (128, 0);
+   test (192, 0);
+   test (255, 0);
+   return 0;
+ }
+

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



More information about the Gcc-patches mailing list