[Bug tree-optimization/114469] New: gcc.dg/torture/bitint-64.c failure with -O2 -flto -std=c23 -fwrapv

[Bug tree-optimization/114469] New: gcc.dg/torture/bitint-64.c failure with -O2 -flto -std=c23 -fwrapv

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114469

            Bug ID: 114469
           Summary: gcc.dg/torture/bitint-64.c failure with -O2 -flto
                    -std=c23 -fwrapv
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

This test fails with
make check-gcc GCC_TEST_RUN_EXPENSIVE=1
RUNTESTFLAGS="dg-torture.exp=bitint-64.c"
FAIL: gcc.dg/torture/bitint-64.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
The question is if _Atomic _BitInt(5) should be in memory always sign extended,
or as the ABI says the upper bits are unspecified.
If the former, then I'd say the problem is on the
/* Strip inner integral conversions that do not change precision or size, or
   zero-extend while keeping the same size (for bool-to-char).  */
(simplify
  (view_convert (convert@0 @1))
  (if ((INTEGRAL_TYPE_P (TREE_TYPE (@0)) || POINTER_TYPE_P (TREE_TYPE (@0)))
       && (INTEGRAL_TYPE_P (TREE_TYPE (@1)) || POINTER_TYPE_P (TREE_TYPE (@1)))
       && TYPE_SIZE (TREE_TYPE (@0)) == TYPE_SIZE (TREE_TYPE (@1))
       && (TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (TREE_TYPE (@1))
           || (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (TREE_TYPE
(@1))
               && TYPE_UNSIGNED (TREE_TYPE (@1)))))
   (view_convert @1)))
match.pd simplification which transforms
  _5 = (unsigned _BitInt(5)) _4;
  _7 = (unsigned _BitInt(5)) e.0_1;
  _8 = _5 + _7;
  _9 = (_BitInt(5)) _8;
  _10 = VIEW_CONVERT_EXPR<unsigned char>(_9);
to just
  _5 = (unsigned _BitInt(5)) _4;
  _7 = (unsigned _BitInt(5)) e.0_1;
  _8 = _5 + _7;
  _10 = VIEW_CONVERT_EXPR<unsigned char>(_8);
so it is no longer sign extended in the unsigned char.
If the upper bits are undefined as the psABI on x86 says, then perhaps the
VIEW_CONVERT_EXPR emitted by c-common.cc (resolve_overloaded_builtin) is the
culprit:
      _13 = __atomic_load_1 (&b, 5);
      _14 = VIEW_CONVERT_EXPR<_BitInt(5)>(_13);
and we should change that
            /* Cast function result from I{1,2,4,8,16} to the required type. 
*/
            result = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (new_return),
result);
to something like if (INTEGRAL_TYPE_P (TREE_TYPE (new_return))IINTEGRAL_TYPE_P
(TREE_TYPE (new_return))NTEGRAL_TYPE_P (TREE_TYPE (new_return)))
            if (INTEGRAL_TYPE_P (TREE_TYPE (new_return)))
              result = fold_convert (TREE_TYPE (new_return), result);
            else
              result = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (new_return),
result);
Joseph, thoughts on this?

[Bug tree-optimization/114469] gcc.dg/torture/bitint-64.c failure with -O2 -flto -std=c23 -fwrapv

[jsm28 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114469

--- Comment #1 from Joseph S. Myers <jsm28 at gcc dot gnu.org> ---
I'd expect _Atomic _BitInt(5) to follow the same ABI (regarding upper bits
being defined or not) as plain _BitInt(5), and any simplification needs to deal
with that.

(In principle for atomics with _BitInt with padding bits there are the same
concerns as for any types with padding bits - any compare-and-exchange loop
needs to succeed eventually when the underlying memory isn't being modified,
rather than failing in comparison of padding bits that might not have been
consistently loaded / preserved. My guess is this is unlikely to be a concern
in practice for _BitInt the way it is e.g. for x86 long double where the
padding bits only exist in memory and not in registers.)

[Bug tree-optimization/114469] gcc.dg/torture/bitint-64.c failure with -O2 -flto -std=c23 -fwrapv

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114469

--- Comment #2 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:4b0443361a82ef89d519c9ae6d4d3bec74376e8f

commit r14-9694-g4b0443361a82ef89d519c9ae6d4d3bec74376e8f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 27 19:38:06 2024 +0100

    c-family: Cast __atomic_load_*/__atomic_exchange_* result to _BitInt rather
then VCE it [PR114469]

    As written in the PR, torture/bitint-64.c test fails with -O2 -flto
    and the reason is that on _BitInt arches where the padding bits
    are undefined, the padding bits in the _Atomic vars are also undefined,
    but when __atomic_load or __atomic_exchange on a _BitInt _Atomic variable
    with some padding bits is lowered into __atomic_load_{1,2,4,8,16} or
    __atomic_exchange_*, the mode precision unsigned result is
VIEW_CONVERT_EXPR
    converted to _BitInt and because of the VCE nothing actually sign/zero
    extends it as needed for later uses - the var is no longer addressable and
    expansion assumes such automatic vars are properly extended.

    The following patch fixes that by using NOP_EXPR on it (the
    VIEW_CONVERT_EXPR after it will then be optimized away during
    gimplification, didn't want to repeat it in the code as else result =
build1
    (VIEW_CONVERT_EXPR, ...); twice.

    2024-03-27  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/114469
            * c-common.cc (resolve_overloaded_builtin): For _BitInt result
            on !extended targets convert result to the _BitInt type before
            using VIEW_CONVERT_EXPR.