]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Fix wrong optimization of complex boolean expression
authorEric Botcazou <ebotcazou@adacore.com>
Fri, 9 May 2025 15:45:27 +0000 (17:45 +0200)
committerEric Botcazou <ebotcazou@adacore.com>
Fri, 9 May 2025 15:49:33 +0000 (17:49 +0200)
The VRP2 pass turns:

  # prephitmp_3 = PHI <0(4)>
  _1 = prephitmp_3 == 0;
  _5 = stretch_14(D) ^ 1;
  _39 = _1 & _5;
  _40 = _39 | last_20(D);

into

  _5 = stretch_14(D) ^ 1;
  _42 = ~stretch_14(D);
  _39 = _42;
  _40 = last_20(D) | _39;

using the following step:

Folding statement: _1 = prephitmp_3 == 0;
Queued stmt for removal.  Folds to: 1
Folding statement: _5 = stretch_14(D) ^ 1;
Not folded
Folding statement: _39 = _1 & _5;
gimple_simplified to _42 = ~stretch_14(D);
_39 = _42 & 1;
Folded into: _39 = _42;

Folding statement: _40 = _39 | last_20(D);
Folded into: _40 = last_20(D) | _39;

but stretch_14 is a 8-bit boolean so the two forms are not equivalent, that
is to say dropping the "& 1" is wrong.  It's another instance of the issue:
  https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558537.html

Here it's the reverse case: the bitwise NOT (~) is treated as logical by the
machinery in range-op.cc but the bitwise AND (&) is *not* treated as logical
by that of vr-values.cc, leading to the same problematic outcome.

gcc/
* vr-values.cc (simplify_using_ranges::simplify) <BIT_AND_EXPR>:
Do not call simplify_bit_ops_using_ranges for boolean types whose
precision is not 1.

gcc/testsuite/
* gnat.dg/opt106.adb: New test.
* gnat.dg/opt106_pkg1.ads, gnat.dg/opt106_pkg1.adb: New helper.
* gnat.dg/opt106_pkg2.ads, gnat.dg/opt106_pkg2.adb: Likewise.

gcc/testsuite/gnat.dg/opt106.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt106_pkg1.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt106_pkg1.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt106_pkg2.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt106_pkg2.ads [new file with mode: 0644]
gcc/vr-values.cc

diff --git a/gcc/testsuite/gnat.dg/opt106.adb b/gcc/testsuite/gnat.dg/opt106.adb
new file mode 100644 (file)
index 0000000..525930b
--- /dev/null
@@ -0,0 +1,11 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Opt106_Pkg1; use Opt106_Pkg1;
+
+procedure Opt106 is
+  Obj : T := (False, 0, 0, 0, True);
+
+begin
+  Proc (Obj, 0, False, True);
+end;
diff --git a/gcc/testsuite/gnat.dg/opt106_pkg1.adb b/gcc/testsuite/gnat.dg/opt106_pkg1.adb
new file mode 100644 (file)
index 0000000..154b13f
--- /dev/null
@@ -0,0 +1,39 @@
+with Opt106_Pkg2; use Opt106_Pkg2;
+
+package body Opt106_Pkg1 is
+
+  procedure Proc (Obj     : in out T;
+                  Data    : Integer;
+                  Last    : Boolean;
+                  Stretch : Boolean) is
+
+  begin
+    if Stretch and then (Obj.Delayed /= 0 or else not Obj.Attach_Last) then
+      raise Program_Error;
+    end if;
+
+    if Obj.Delayed /= 0 then
+      Stop (Obj.Delayed, Obj.Before, Data, False);
+    end if;
+
+    if Last or (Obj.Delayed = 0 and not Stretch) then
+      Stop (Data, Obj.Before, 0, Last);
+
+      if Last then
+        Obj.Initialized := False;
+      else
+        Obj.Next := 0;
+        Obj.Before := Data;
+      end if;
+
+    else
+      if Stretch then
+        Obj.Next := 1;
+      else
+        Obj.Before := Obj.Delayed;
+      end if;
+      Obj.Delayed := Data;
+    end if;
+  end;
+
+end Opt106_Pkg1;
diff --git a/gcc/testsuite/gnat.dg/opt106_pkg1.ads b/gcc/testsuite/gnat.dg/opt106_pkg1.ads
new file mode 100644 (file)
index 0000000..85ac24d
--- /dev/null
@@ -0,0 +1,16 @@
+package Opt106_Pkg1 is
+
+  type T is record
+    Initialized   : Boolean;
+    Before        : Integer;
+    Delayed       : Integer;
+    Next          : Integer;
+    Attach_Last   : Boolean;
+  end record;
+
+  procedure Proc (Obj     : in out T;
+                  Data    : Integer;
+                  Last    : Boolean;
+                  Stretch : Boolean);
+
+end Opt106_Pkg1;
diff --git a/gcc/testsuite/gnat.dg/opt106_pkg2.adb b/gcc/testsuite/gnat.dg/opt106_pkg2.adb
new file mode 100644 (file)
index 0000000..cf63956
--- /dev/null
@@ -0,0 +1,11 @@
+package body Opt106_Pkg2 is
+
+  procedure Stop (Delayed : Integer;
+                  Before  : Integer;
+                  After   : Integer;
+                  Last    : Boolean) is
+  begin
+     raise Program_Error;
+  end;
+
+end Opt106_Pkg2;
diff --git a/gcc/testsuite/gnat.dg/opt106_pkg2.ads b/gcc/testsuite/gnat.dg/opt106_pkg2.ads
new file mode 100644 (file)
index 0000000..77e5b40
--- /dev/null
@@ -0,0 +1,8 @@
+package Opt106_Pkg2 is
+
+  procedure Stop (Delayed : Integer;
+                  Before  : Integer;
+                  After   : Integer;
+                  Last    : Boolean);
+
+end Opt106_Pkg2;
index 6603d90c392c83cf74e07ea4bf281ba287460ed0..4c787593b95a7c12e6e74ad486471145464132db 100644 (file)
@@ -1996,10 +1996,13 @@ simplify_using_ranges::simplify (gimple_stmt_iterator *gsi)
 
        case BIT_AND_EXPR:
        case BIT_IOR_EXPR:
-         /* Optimize away BIT_AND_EXPR and BIT_IOR_EXPR
-            if all the bits being cleared are already cleared or
-            all the bits being set are already set.  */
-         if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+         /* Optimize away BIT_AND_EXPR and BIT_IOR_EXPR if all the bits
+            being cleared are already cleared or all the bits being set
+            are already set.  Beware that boolean types must be handled
+            logically (see range-op.cc) unless they have precision 1.  */
+         if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1))
+             && (TREE_CODE (TREE_TYPE (rhs1)) != BOOLEAN_TYPE
+                 || TYPE_PRECISION (TREE_TYPE (rhs1)) == 1))
            return simplify_bit_ops_using_ranges (gsi, stmt);
          break;