* [PATCH 1/2] Fix PR 110487: invalid signed boolean value @ 2023-07-01 8:22 Andrew Pinski 2023-07-01 8:22 ` [PATCH 2/2] PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety Andrew Pinski 2023-07-03 7:50 ` [PATCH 1/2] Fix PR 110487: invalid signed boolean value Richard Biener 0 siblings, 2 replies; 4+ messages in thread From: Andrew Pinski @ 2023-07-01 8:22 UTC (permalink / raw) To: gcc-patches; +Cc: Andrew Pinski This fixes the first part of this bug where `a ? -1 : 0` would cause a value of 1 into the signed boolean value. It fixes the problem by casting to an integer type of the same size/signedness before doing the negative and then casting to the type of expression. OK? Bootstrapped and tested on x86_64. gcc/ChangeLog: * match.pd (a?-1:0): Cast type an integer type rather the type before the negative. (a?0:-1): Likewise. --- gcc/match.pd | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/gcc/match.pd b/gcc/match.pd index 45c72e733a5..a0d114f6a16 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -4703,7 +4703,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* a ? -1 : 0 -> -a. No need to check the TYPE_PRECISION not being 1 here as the powerof2cst case above will handle that case correctly. */ (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1)) - (negate (convert (convert:boolean_type_node @0)))))) + (with { + auto prec = TYPE_PRECISION (type); + auto unsign = TYPE_UNSIGNED (type); + tree inttype = build_nonstandard_integer_type (prec, unsign); + } + (convert (negate (convert:inttype (convert:boolean_type_node @0)))))))) (if (integer_zerop (@1)) (with { tree booltrue = constant_boolean_node (true, boolean_type_node); @@ -4722,7 +4727,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* a ? -1 : 0 -> -(!a). No need to check the TYPE_PRECISION not being 1 here as the powerof2cst case above will handle that case correctly. */ (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2)) - (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))) + (with { + auto prec = TYPE_PRECISION (type); + auto unsign = TYPE_UNSIGNED (type); + tree inttype = build_nonstandard_integer_type (prec, unsign); + } + (convert + (negate + (convert:inttype + (bit_xor (convert:boolean_type_node @0) { booltrue; } ) + ) + ) + ) + ) + ) ) ) ) -- 2.31.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety 2023-07-01 8:22 [PATCH 1/2] Fix PR 110487: invalid signed boolean value Andrew Pinski @ 2023-07-01 8:22 ` Andrew Pinski 2023-07-04 8:55 ` Richard Biener 2023-07-03 7:50 ` [PATCH 1/2] Fix PR 110487: invalid signed boolean value Richard Biener 1 sibling, 1 reply; 4+ messages in thread From: Andrew Pinski @ 2023-07-01 8:22 UTC (permalink / raw) To: gcc-patches; +Cc: Andrew Pinski The problem here is we might produce some values out of the type's min/max (and/or valid values, e.g. signed booleans). The fix is to use an integer type which has the same precision and signedness as the original type. Note two_value_replacement in phiopt had the same issue in previous versions; though I don't know if a problem will show up there. OK? Bootstrapped and tested on x86_64-linux-gnu. gcc/ChangeLog: PR tree-optimization/110487 * match.pd (a !=/== CST1 ? CST2 : CST3): Always build a nonstandard integer and use that. --- gcc/match.pd | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/gcc/match.pd b/gcc/match.pd index a0d114f6a16..9748ad8466e 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -4797,24 +4797,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) tree type1; if ((eqne == EQ_EXPR) ^ (wi::to_wide (@1) == min)) std::swap (arg0, arg1); - if (TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (type)) - { - /* Avoid performing the arithmetics in bool type which has different - semantics, otherwise prefer unsigned types from the two with - the same precision. */ - if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE - || !TYPE_UNSIGNED (type)) - type1 = TREE_TYPE (@0); - else - type1 = TREE_TYPE (arg0); - } - else if (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (type)) + if (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (type)) type1 = TREE_TYPE (@0); else type1 = type; - min = wide_int::from (min, TYPE_PRECISION (type1), + auto prec = TYPE_PRECISION (type1); + auto unsign = TYPE_UNSIGNED (type1); + type1 = build_nonstandard_integer_type (prec, unsign); + min = wide_int::from (min, prec, TYPE_SIGN (TREE_TYPE (@0))); - wide_int a = wide_int::from (wi::to_wide (arg0), TYPE_PRECISION (type1), + wide_int a = wide_int::from (wi::to_wide (arg0), prec, TYPE_SIGN (type)); enum tree_code code; wi::overflow_type ovf; @@ -4822,7 +4814,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) { code = PLUS_EXPR; a -= min; - if (!TYPE_UNSIGNED (type1)) + if (!unsign) { /* lhs is known to be in range [min, min+1] and we want to add a to it. Check if that operation can overflow for those 2 values @@ -4836,7 +4828,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) { code = MINUS_EXPR; a += min; - if (!TYPE_UNSIGNED (type1)) + if (!unsign) { /* lhs is known to be in range [min, min+1] and we want to subtract it from a. Check if that operation can overflow for those 2 -- 2.31.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety 2023-07-01 8:22 ` [PATCH 2/2] PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety Andrew Pinski @ 2023-07-04 8:55 ` Richard Biener 0 siblings, 0 replies; 4+ messages in thread From: Richard Biener @ 2023-07-04 8:55 UTC (permalink / raw) To: Andrew Pinski; +Cc: gcc-patches On Sat, Jul 1, 2023 at 10:23 AM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > The problem here is we might produce some values out of the type's > min/max (and/or valid values, e.g. signed booleans). The fix is to > use an integer type which has the same precision and signedness > as the original type. > > Note two_value_replacement in phiopt had the same issue in previous > versions; though I don't know if a problem will show up there. > > OK? Bootstrapped and tested on x86_64-linux-gnu. OK. > gcc/ChangeLog: > > PR tree-optimization/110487 > * match.pd (a !=/== CST1 ? CST2 : CST3): Always > build a nonstandard integer and use that. > --- > gcc/match.pd | 24 ++++++++---------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/gcc/match.pd b/gcc/match.pd > index a0d114f6a16..9748ad8466e 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -4797,24 +4797,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > tree type1; > if ((eqne == EQ_EXPR) ^ (wi::to_wide (@1) == min)) > std::swap (arg0, arg1); > - if (TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (type)) > - { > - /* Avoid performing the arithmetics in bool type which has different > - semantics, otherwise prefer unsigned types from the two with > - the same precision. */ > - if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE > - || !TYPE_UNSIGNED (type)) > - type1 = TREE_TYPE (@0); > - else > - type1 = TREE_TYPE (arg0); > - } > - else if (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (type)) > + if (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (type)) > type1 = TREE_TYPE (@0); > else > type1 = type; > - min = wide_int::from (min, TYPE_PRECISION (type1), > + auto prec = TYPE_PRECISION (type1); > + auto unsign = TYPE_UNSIGNED (type1); > + type1 = build_nonstandard_integer_type (prec, unsign); > + min = wide_int::from (min, prec, > TYPE_SIGN (TREE_TYPE (@0))); > - wide_int a = wide_int::from (wi::to_wide (arg0), TYPE_PRECISION (type1), > + wide_int a = wide_int::from (wi::to_wide (arg0), prec, > TYPE_SIGN (type)); > enum tree_code code; > wi::overflow_type ovf; > @@ -4822,7 +4814,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > { > code = PLUS_EXPR; > a -= min; > - if (!TYPE_UNSIGNED (type1)) > + if (!unsign) > { > /* lhs is known to be in range [min, min+1] and we want to add a > to it. Check if that operation can overflow for those 2 values > @@ -4836,7 +4828,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > { > code = MINUS_EXPR; > a += min; > - if (!TYPE_UNSIGNED (type1)) > + if (!unsign) > { > /* lhs is known to be in range [min, min+1] and we want to subtract > it from a. Check if that operation can overflow for those 2 > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] Fix PR 110487: invalid signed boolean value 2023-07-01 8:22 [PATCH 1/2] Fix PR 110487: invalid signed boolean value Andrew Pinski 2023-07-01 8:22 ` [PATCH 2/2] PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety Andrew Pinski @ 2023-07-03 7:50 ` Richard Biener 1 sibling, 0 replies; 4+ messages in thread From: Richard Biener @ 2023-07-03 7:50 UTC (permalink / raw) To: Andrew Pinski; +Cc: gcc-patches On Sat, Jul 1, 2023 at 10:23 AM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > This fixes the first part of this bug where `a ? -1 : 0` > would cause a value of 1 into the signed boolean value. > It fixes the problem by casting to an integer type of > the same size/signedness before doing the negative and > then casting to the type of expression. > > OK? Bootstrapped and tested on x86_64. OK. Richard. > gcc/ChangeLog: > > * match.pd (a?-1:0): Cast type an integer type > rather the type before the negative. > (a?0:-1): Likewise. > --- > gcc/match.pd | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/gcc/match.pd b/gcc/match.pd > index 45c72e733a5..a0d114f6a16 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -4703,7 +4703,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* a ? -1 : 0 -> -a. No need to check the TYPE_PRECISION not being 1 > here as the powerof2cst case above will handle that case correctly. */ > (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1)) > - (negate (convert (convert:boolean_type_node @0)))))) > + (with { > + auto prec = TYPE_PRECISION (type); > + auto unsign = TYPE_UNSIGNED (type); > + tree inttype = build_nonstandard_integer_type (prec, unsign); > + } > + (convert (negate (convert:inttype (convert:boolean_type_node @0)))))))) > (if (integer_zerop (@1)) > (with { > tree booltrue = constant_boolean_node (true, boolean_type_node); > @@ -4722,7 +4727,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* a ? -1 : 0 -> -(!a). No need to check the TYPE_PRECISION not being 1 > here as the powerof2cst case above will handle that case correctly. */ > (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2)) > - (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))) > + (with { > + auto prec = TYPE_PRECISION (type); > + auto unsign = TYPE_UNSIGNED (type); > + tree inttype = build_nonstandard_integer_type (prec, unsign); > + } > + (convert > + (negate > + (convert:inttype > + (bit_xor (convert:boolean_type_node @0) { booltrue; } ) > + ) > + ) > + ) > + ) > + ) > ) > ) > ) > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-04 8:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-01 8:22 [PATCH 1/2] Fix PR 110487: invalid signed boolean value Andrew Pinski 2023-07-01 8:22 ` [PATCH 2/2] PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety Andrew Pinski 2023-07-04 8:55 ` Richard Biener 2023-07-03 7:50 ` [PATCH 1/2] Fix PR 110487: invalid signed boolean value Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).