diff options
author | Eric Botcazou <ebotcazou@adacore.com> | 2025-05-09 17:45:27 +0200 |
---|---|---|
committer | Eric Botcazou <ebotcazou@adacore.com> | 2025-05-09 19:24:49 +0200 |
commit | 6683f2cd1ce07fc8470b19dc3c1578b3e0645c25 (patch) | |
tree | 3470dcceba1aa8e99bc501f7071ac4688d66dc98 | |
parent | 89ca647487b596cf1dda14075224d000fa542897 (diff) | |
download | gcc-6683f2cd1ce07fc8470b19dc3c1578b3e0645c25.zip gcc-6683f2cd1ce07fc8470b19dc3c1578b3e0645c25.tar.gz gcc-6683f2cd1ce07fc8470b19dc3c1578b3e0645c25.tar.bz2 |
Fix wrong optimization of complex boolean expression
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.
-rw-r--r-- | gcc/testsuite/gnat.dg/opt106.adb | 11 | ||||
-rw-r--r-- | gcc/testsuite/gnat.dg/opt106_pkg1.adb | 39 | ||||
-rw-r--r-- | gcc/testsuite/gnat.dg/opt106_pkg1.ads | 16 | ||||
-rw-r--r-- | gcc/testsuite/gnat.dg/opt106_pkg2.adb | 11 | ||||
-rw-r--r-- | gcc/testsuite/gnat.dg/opt106_pkg2.ads | 8 | ||||
-rw-r--r-- | gcc/vr-values.cc | 11 |
6 files changed, 92 insertions, 4 deletions
diff --git a/gcc/testsuite/gnat.dg/opt106.adb b/gcc/testsuite/gnat.dg/opt106.adb new file mode 100644 index 0000000..525930b --- /dev/null +++ b/gcc/testsuite/gnat.dg/opt106.adb @@ -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 index 0000000..154b13f --- /dev/null +++ b/gcc/testsuite/gnat.dg/opt106_pkg1.adb @@ -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 index 0000000..85ac24d --- /dev/null +++ b/gcc/testsuite/gnat.dg/opt106_pkg1.ads @@ -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 index 0000000..cf63956 --- /dev/null +++ b/gcc/testsuite/gnat.dg/opt106_pkg2.adb @@ -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 index 0000000..77e5b40 --- /dev/null +++ b/gcc/testsuite/gnat.dg/opt106_pkg2.ads @@ -0,0 +1,8 @@ +package Opt106_Pkg2 is + + procedure Stop (Delayed : Integer; + Before : Integer; + After : Integer; + Last : Boolean); + +end Opt106_Pkg2; diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc index 6603d90..4c78759 100644 --- a/gcc/vr-values.cc +++ b/gcc/vr-values.cc @@ -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; |