[ranger] Fix wrong code for boolean negation in condition at -O

[ranger] Fix wrong code for boolean negation in condition at -O

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

Hi,

this is a regression present on mainline in the form of wrong code generated 
for the attached Ada testcase at -O -ftree-vrp.  It's again an issue with the 
8-bit booleans in Ada and, as a matter of fact, it's exactly the same issue as 
the one I fixed elsewhere about one year and half ago at:
  https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517843.html

The problem is the bitwise/logical dichotomy for operators and the transition 
from the former to the latter for boolean types: if they are 1-bit, that's 
straightforward but, if they are larger, then you need to be careful because 
you cannot, on the one hand, turn a bitwise AND into a logical AND and, on the 
other hand, *not* turn e.g. a bitwise NOT into a logical NOT if they occur in 
the same computation, as the first change will drop the masking that may need 
to be applied after the bitwise NOT if it is not also changed.

Given that the ranger turns bitwise AND/OR into logical AND/OR for booleans, 
the patch does the same for bitwise NOT, exactly as in the above fix.

Bootstrapped/regtested on x86-64/Linux, OK for the mainline?


2020-11-09  Eric Botcazou  <ebotcazou@adacore.com>

	* range-op.cc (operator_logical_not::fold_range): Tidy up.
	(operator_logical_not::op1_range): Call above method.
	(operator_bitwise_not::fold_range): If the type is compatible
	with boolean, call op_logical_not.fold_range.
	(operator_bitwise_not::op1_range): If the type is compatible
	with boolean, call op_logical_not.op1_range.


2020-11-09  Eric Botcazou  <ebotcazou@adacore.com>

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

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1696 bytes --]

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index f38f02e8d27..238bf327d9f 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -2706,27 +2706,21 @@ operator_logical_not::fold_range (irange &r, tree type,
   if (empty_range_varying (r, type, lh, rh))
     return true;
 
-  if (lh.varying_p () || lh.undefined_p ())
-    r = lh;
-  else
-    {
-      r = lh;
-      r.invert ();
-    }
-  gcc_checking_assert (lh.type() == type);
+  r = lh;
+  if (!lh.varying_p () && !lh.undefined_p ())
+    r.invert ();
+
   return true;
 }
 
 bool
 operator_logical_not::op1_range (irange &r,
-				 tree type ATTRIBUTE_UNUSED,
+				 tree type,
 				 const irange &lhs,
-				 const irange &op2 ATTRIBUTE_UNUSED) const
+				 const irange &op2) const
 {
-  r = lhs;
-  if (!lhs.varying_p () && !lhs.undefined_p ())
-    r.invert ();
-  return true;
+  // Logical NOT is involutary...do it again.
+  return fold_range (r, type, lhs, op2);
 }
 
 
@@ -2749,6 +2743,9 @@ operator_bitwise_not::fold_range (irange &r, tree type,
   if (empty_range_varying (r, type, lh, rh))
     return true;
 
+  if (types_compatible_p (type, boolean_type_node))
+    return op_logical_not.fold_range (r, type, lh, rh);
+
   // ~X is simply -1 - X.
   int_range<1> minusone (type, wi::minus_one (TYPE_PRECISION (type)),
 			 wi::minus_one (TYPE_PRECISION (type)));
@@ -2761,6 +2758,9 @@ operator_bitwise_not::op1_range (irange &r, tree type,
 				 const irange &lhs,
 				 const irange &op2) const
 {
+  if (types_compatible_p (type, boolean_type_node))
+    return op_logical_not.op1_range (r, type, lhs, op2);
+
   // ~X is -1 - X and since bitwise NOT is involutary...do it again.
   return fold_range (r, type, lhs, op2);
 }

[-- Attachment #3: opt88.adb --]
[-- Type: text/x-adasrc, Size: 716 bytes --]

-- { dg-do run }
-- { dg-options "-O -ftree-vrp -fno-inline" }

procedure Opt88 is

  Val : Integer := 1;

  procedure Dummy (B : out Boolean) is
  begin
    B := True;
  end;

  function Test return Boolean is
  begin
    return False;
  end;

  procedure Do_It (OK : out Boolean) is

    Blue : Boolean := False;
    Red  : Boolean := False;

  begin
    OK := True;
    Blue := True;
    Dummy (Red);

    if Red then
      Red := False;

      if Test then
        Dummy (Red);
      end if;
    end if;

    if Blue and not Red then
      Val := 0;
    end if;

    if Red then
      OK := False;
    end if;
  end;

  OK : Boolean;

begin
  Do_It (OK);
  if not OK then
    raise Program_Error;
  end if;
end;

Re: [ranger] Fix wrong code for boolean negation in condition at -O

[Andrew MacLeod]

On 11/9/20 4:38 PM, Eric Botcazou wrote:
> Hi,
>
> this is a regression present on mainline in the form of wrong code generated
> for the attached Ada testcase at -O -ftree-vrp.  It's again an issue with the
> 8-bit booleans in Ada and, as a matter of fact, it's exactly the same issue as
> the one I fixed elsewhere about one year and half ago at:
>    https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517843.html
>
> The problem is the bitwise/logical dichotomy for operators and the transition
> from the former to the latter for boolean types: if they are 1-bit, that's
> straightforward but, if they are larger, then you need to be careful because
> you cannot, on the one hand, turn a bitwise AND into a logical AND and, on the
> other hand, *not* turn e.g. a bitwise NOT into a logical NOT if they occur in
> the same computation, as the first change will drop the masking that may need
> to be applied after the bitwise NOT if it is not also changed.
>
> Given that the ranger turns bitwise AND/OR into logical AND/OR for booleans,
> the patch does the same for bitwise NOT, exactly as in the above fix.
>
> Bootstrapped/regtested on x86-64/Linux, OK for the mainline?
>
>
> 2020-11-09  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* range-op.cc (operator_logical_not::fold_range): Tidy up.
> 	(operator_logical_not::op1_range): Call above method.
> 	(operator_bitwise_not::fold_range): If the type is compatible
> 	with boolean, call op_logical_not.fold_range.
> 	(operator_bitwise_not::op1_range): If the type is compatible
> 	with boolean, call op_logical_not.op1_range.
>

Doh. Sorry about that. The project has a long painful history with multi 
bit booleans :-P.. Hopefully this is the last of them :-)

THanks.

OK.

Andrew