diff mbox series

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

Message ID 6981748.0S6LrZOK0d@fomalhaut
State New
Headers show
Series [ranger] Fix wrong code for boolean negation in condition at -O | expand

Commit Message

Eric Botcazou Nov. 9, 2020, 9:38 p.m. UTC 
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.

Comments

Andrew MacLeod Nov. 9, 2020, 10:37 p.m. UTC | #1 
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
diff mbox series

Patch

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);
 }