On Thu, Jan 23, 2025 at 8:43 AM Alexandre Oliva <ol...@adacore.com> wrote:
>
>
> When comparing a signed narrow variable with a wider constant that has
> the bit corresponding to the variable's sign bit set, we would check
> that the constant is a sign-extension from that sign bit, and conclude
> that the compare fails if it isn't.
>
> When the signed variable is masked without getting the [lr]l_signbit
> variable set, or when the sign bit itself is masked out, we know the
> sign-extension bits from the extended variable are going to be zero,
> so the constant will only compare equal if it is a zero- rather than
> sign-extension from the narrow variable's precision, therefore, check
> that it satisfies this property, and yield a false compare result
> otherwise.
OK.
Thanks,
Richard.
>
> for gcc/ChangeLog
>
> PR tree-optimization/118572
> * gimple-fold.cc (fold_truth_andor_for_ifcombine): Compare as
> unsigned the variables whose extension bits are masked out.
>
> for gcc/testsuite/ChangeLog
>
> PR tree-optimization/118572
> * gcc.dg/field-merge-24.c: New.
> ---
> gcc/gimple-fold.cc | 20 ++++++++++++++++--
> gcc/testsuite/gcc.dg/field-merge-24.c | 36
> +++++++++++++++++++++++++++++++++
> 2 files changed, 53 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/field-merge-24.c
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index cd9d350349186..13541fe1ef749 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -8552,12 +8552,21 @@ fold_truth_andor_for_ifcombine (enum tree_code code,
> tree truth_type,
> {
> /* Before clipping upper bits of the right-hand operand of the compare,
> check that they're sign or zero extensions, depending on how the
> - left-hand operand would be extended. */
> + left-hand operand would be extended. If it is unsigned, or if
> there's
> + a mask that zeroes out extension bits, whether because we've checked
> + for upper bits in the mask and did not set ll_signbit, or because the
> + sign bit itself is masked out, check that the right-hand operand is
> + zero-extended. */
> bool l_non_ext_bits = false;
> if (ll_bitsize < lr_bitsize)
> {
> wide_int zext = wi::zext (l_const, ll_bitsize);
> - if ((ll_unsignedp ? zext : wi::sext (l_const, ll_bitsize)) ==
> l_const)
> + if ((ll_unsignedp
> + || (ll_and_mask.get_precision ()
> + && (!ll_signbit
> + || ((ll_and_mask & wi::mask (ll_bitsize - 1, true,
> ll_bitsize))
> + == 0)))
> + ? zext : wi::sext (l_const, ll_bitsize)) == l_const)
> l_const = zext;
> else
> l_non_ext_bits = true;
> @@ -8583,7 +8592,12 @@ fold_truth_andor_for_ifcombine (enum tree_code code,
> tree truth_type,
> if (rl_bitsize < rr_bitsize)
> {
> wide_int zext = wi::zext (r_const, rl_bitsize);
> - if ((rl_unsignedp ? zext : wi::sext (r_const, rl_bitsize)) ==
> r_const)
> + if ((rl_unsignedp
> + || (rl_and_mask.get_precision ()
> + && (!rl_signbit
> + || ((rl_and_mask & wi::mask (rl_bitsize - 1, true,
> rl_bitsize))
> + == 0)))
> + ? zext : wi::sext (r_const, rl_bitsize)) == r_const)
> r_const = zext;
> else
> r_non_ext_bits = true;
> diff --git a/gcc/testsuite/gcc.dg/field-merge-24.c
> b/gcc/testsuite/gcc.dg/field-merge-24.c
> new file mode 100644
> index 0000000000000..ce5bce7d0b49c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/field-merge-24.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +/* PR tree-optimization/118572 */
> +/* Check that signed compares with constants that seem signed in the other
> + compare operand's width get treated as unsigned if its upper bits are
> masked
> + out. */
> +
> +__attribute__((noipa))
> +int test(signed char c)
> +{
> + return (((0x80 & (c&0xff)) != 0) && ((0xc0 & (c&0xff)) == 0x80));
> +}
> +
> +__attribute__((noipa))
> +int test2(signed char c)
> +{
> + return (((-128 & (c&-1)) != 0) && ((-64 & (c&-1)) == -128));
> +}
> +
> +__attribute__((noipa))
> +int test3(signed char c)
> +{
> + return (((0x80 & (c&-1)) != 0) && ((0x1248c0 & (c&-1)) == 0x124880));
> +}
> +
> +__attribute__((noipa))
> +int test4(signed char c)
> +{
> + return (((0x400 & (c&-1)) == 0) && ((0x40 & (c&-1)) == 0x40));
> +}
> +
> +int main() {
> + if (test(0x80) == 0 || test2(-128) == 0 || test3(-128) == 0 || test4(64)
> == 0)
> + __builtin_abort();
> +}
>
>
> --
> Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
> Free Software Activist GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
