diff mbox

Fix wrong code with boolean negation

Message ID 10788105.o7paIdRnUj@polaris
State New
Headers show

Commit Message

Eric Botcazou Jan. 21, 2013, 8:41 a.m. UTC
Hi,

this is a regression present in the Ada compiler since 4.5: the issue had been 
latent for ages, but an unrelated streamlining of the IR made it appear.

When make_range_step is invoked on:

  (integer)!b < 0

where b is a boolean, it returns "always true" instead of "always false".

The sequence is as follows:

  (integer)!b < 0   is_true_if   not in [0:0]
  (integer)!b       is_true_if   not in [0;+inf[
  !b                is_true_if   not in [0;255]
  b                 is_true_if       in [0;255]

The wrong step is the last one: when TRUTH_NOT_EXPR is seen in make_range_step 
the "in" value is unconditionally toggled.  Of course that doesn't work in the 
general case, just if the range is the "boolean" range.  As a matter of fact, 
this is explained just below for the comparison operators:

      /* We can only do something if the range is testing for zero
	 and if the second operand is an integer constant.  Note that
	 saying something is "in" the range we make is done by
	 complementing IN_P since it will set in the initial case of
	 being not equal to zero; "out" is leaving it alone.  */

so the fix is to use the zero range condition in the TRUTH_NOT_EXPR case.

Tested on x86_64-suse-linux, OK for mainline?  And for branch(es)?


2013-01-21  Eric Botcazou  <ebotcazou@adacore.com>

	* fold-const.c (make_range_step) <TRUTH_NOT_EXPR>: Bail out if the
	range isn't testing for zero.


2013-01-21  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/opt26.adb: New test.

Comments

Richard Biener Feb. 1, 2013, 9:10 a.m. UTC | #1
On Mon, Jan 21, 2013 at 9:41 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is a regression present in the Ada compiler since 4.5: the issue had been
> latent for ages, but an unrelated streamlining of the IR made it appear.
>
> When make_range_step is invoked on:
>
>   (integer)!b < 0
>
> where b is a boolean, it returns "always true" instead of "always false".
>
> The sequence is as follows:
>
>   (integer)!b < 0   is_true_if   not in [0:0]
>   (integer)!b       is_true_if   not in [0;+inf[
>   !b                is_true_if   not in [0;255]
>   b                 is_true_if       in [0;255]
>
> The wrong step is the last one: when TRUTH_NOT_EXPR is seen in make_range_step
> the "in" value is unconditionally toggled.  Of course that doesn't work in the
> general case, just if the range is the "boolean" range.  As a matter of fact,
> this is explained just below for the comparison operators:
>
>       /* We can only do something if the range is testing for zero
>          and if the second operand is an integer constant.  Note that
>          saying something is "in" the range we make is done by
>          complementing IN_P since it will set in the initial case of
>          being not equal to zero; "out" is leaving it alone.  */
>
> so the fix is to use the zero range condition in the TRUTH_NOT_EXPR case.
>
> Tested on x86_64-suse-linux, OK for mainline?  And for branch(es)?

Ok everywhere.

Thanks,
Richard.

>
> 2013-01-21  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * fold-const.c (make_range_step) <TRUTH_NOT_EXPR>: Bail out if the
>         range isn't testing for zero.
>
>
> 2013-01-21  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/opt26.adb: New test.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 195310)
+++ fold-const.c	(working copy)
@@ -3813,6 +3813,10 @@  make_range_step (location_t loc, enum tr
   switch (code)
     {
     case TRUTH_NOT_EXPR:
+      /* We can only do something if the range is testing for zero.  */
+      if (low == NULL_TREE || high == NULL_TREE
+	  || ! integer_zerop (low) || ! integer_zerop (high))
+	return NULL_TREE;
       *p_in_p = ! in_p;
       return arg0;