On 9/5/23 01:46, Andrew Pinski wrote:
On Tue, Sep 5, 2023 at 12:09 AM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
On 9/1/23 20:32, Andrew Pinski via Gcc-patches wrote:
This turns out to be a latent bug in ssa_name_has_boolean_range
where it would return true for all boolean types but all of the
uses of ssa_name_has_boolean_range was expecting 0/1 as the range
rather than [-1,0].
So when I fixed vector lower to do all comparisons in boolean_type
rather than still in the signed-boolean:31 type (to fix a different issue),
the pattern in match for `-(type)!A -> (type)A - 1.` would assume A (which
was signed-boolean:31) had a range of [0,1] which broke down and sometimes
gave us -1/-2 as values rather than what we were expecting of -1/0.
This was the simpliest patch I found while testing.
We have another way of matching [0,1] range which we could use instead
of ssa_name_has_boolean_range except that uses only the global ranges
rather than the local range (during VRP).
I tried to clean this up slightly by using gimple_match_zero_one_valuedp
inside ssa_name_has_boolean_range but that failed because due to using
only the global ranges. I then tried to change get_nonzero_bits to use
the local ranges at the optimization time but that failed also because
we would remove branches to __builtin_unreachable during evrp and lose
information as we don't set the global ranges during evrp.
OK? Bootstrapped and tested on x86_64-linux-gnu.
PR 110817
gcc/ChangeLog:
* tree-ssanames.cc (ssa_name_has_boolean_range): Remove the
check for boolean type as they don't have "[0,1]" range.
gcc/testsuite/ChangeLog:
* gcc.c-torture/execute/pr110817-1.c: New test.
* gcc.c-torture/execute/pr110817-2.c: New test.
* gcc.c-torture/execute/pr110817-3.c: New test.
I'm a bit surprised this didn't trigger any regressions. Though maybe
all the existing testcases were capturing cases where non-boolean types
were known to have a 0/1 value.
Well except ssa_name_has_boolean_range will return true for `An
[unsigned] integral type with a single bit of precision` which the
normal boolean type for C is. So the only case where this makes a
difference is signed booleans. Vectors and Ada are the only 2 places I
know of which use signed booleans even.
Ah. That would explain things well. When we added that stuff we were
mostly focused on cases that would have fallen under unsigned integral
types with a single bit of precision.
The other uses of ssa_name_has_boolean_range are in DOM.
Right. That was the original client of this code.
The first 2 uses of ssa_name_has_boolean_range use
build_one_cst/build_one_cst which is definitely wrong there. should
have been constant_boolean_node for N-bit signed boolean types.
ACK. Feel free to fix these. Consider it pre-approved if it passes
testing.
jeff
