public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1
@ 2024-07-25 21:21 laria at laria dot me
  2024-07-25 21:46 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization pinskia at gcc dot gnu.org
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: laria at laria dot me @ 2024-07-25 21:21 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 116098
           Summary: _Bool value from tagged union is incorrect when built
                    with -O1
           Product: gcc
           Version: 14.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: laria at laria dot me
  Target Milestone: ---

Created attachment 58762
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58762&action=edit
preprocessed test-O1.i

I have encountered a weird behavior that seems like a bug in GCC to me.

I have a tagged union representing either a number (int) or a boolean (_Bool)
and a function that determines the "truthyness" of the value (true for all
numbers, the boolean value for booleans).

When I use this on a number and then negate the result, I get the wrong result,
when I compile this with -O1, but the expected result when compiling with -O0
or -O2.

Additionally when I return from main with `return res ? 0 : 1;`, I get neither
0
nor 1 as the exit code, but I instead get 3. (Also only with -O1).

======= BEGIN test-O1.c =======
int puts(const char *);

struct Value {
    enum ValueType {
        VALUE_BOOLEAN,
        VALUE_NUM,
    } type;
    union {
        _Bool boolean;
        int num;
        void *blank[2];
    };
};

static struct Value s_value;
static _Bool s_b;

_Bool
truthy(void)
{
    struct Value value = s_value;
    if (s_b) s_b = 0;
    // Will not reproduce when using an if or a ternary ?: instead.
    switch (value.type) {
    case VALUE_BOOLEAN:
        return value.boolean;
    default:
        return 1;
    }
}

int
main(void)
{
    s_b = 0;
    s_value = (struct Value) {
        .type = VALUE_NUM,

        // Seems to "work" with any value >= 2.
        // Tweak this number to get different return exit codes.
        .num = 2,
    };
    s_value = (struct Value) {
        .type = VALUE_BOOLEAN,
        .boolean = !truthy(), // truthy should be 1, so .boolean=0
    };
    _Bool b = truthy();
    puts(b ? "true" : "false"); // Should print "false", prints "true" instead
    return b ? 0 : 1; // Should return 1, returns 3 instead
}
======= END test-O1.c =======

As far as I can tell, I don't rely on any undefined behavior here. I only
retrieve the value from the union with the member I used to write to it the
last
time, which I make sure of using the stored tag / type.

The preprocessed *.i file for this is also attached.

The full compilation command is:

gcc -Wall -Werror -Wextra -pedantic -std=c11 -fno-strict-aliasing -fwrapv \
    -fno-aggressive-loop-optimizations -O1 test-O1.c

There are no compiler warnings / errors. The compiler exits successfully.

I'm running GCC on x86_64; GNU/Linux (6.9.8-200.fc40.x86_64; Fedora 40)

Compiling with -fsanitize=undefined does not produce runtime errors, but the
bug
then goes away.

Strangely enough a slightly less reduced source shows the same behavior, but
now
it only happens with -O2, not with -O1/-O0:

======= BEGIN test-O2.c =======
int puts(const char *);

struct Value {
    enum ValueType {
        VALUE_BOOLEAN,
        VALUE_NUM,
    } type;
    union {
        _Bool boolean;
        int num;
        void *blank[2];
    };
};

static struct Value s_value;
static _Bool s_b;

static void
val_set(struct Value value)
{
    s_b = 0;
    s_value = value;
}

static struct Value
val_get(void)
{
    struct Value value = s_value;
    if (s_b) s_b = 0;
    return value;
}

static _Bool
truthy(void)
{
    struct Value value = val_get();
    // Will not reproduce when using an if or a ternary ?: instead.
    switch (value.type) {
    case VALUE_BOOLEAN:
        return value.boolean;
    default:
        return 1;
    }
}

int
main(void)
{
    s_b = 0;
    val_set((struct Value) {
        .type = VALUE_NUM,

        // Seems to "work" with any value >= 2.
        // Tweak this number to get different return exit codes.
        .num = 2,
    });
    _Bool b1 = truthy(); // truthy should be 1
    val_set((struct Value) {
        .type = VALUE_BOOLEAN,
        .boolean = !b1, // SHould be 0
    });
    _Bool b2 = truthy();
    puts(b2 ? "true" : "false"); // Should print "false", prints "true" instead
    return b2 ? 0 : 1; // Should return 1, returns 3 instead
}
======= END test-O2.c =======

I have tested this with ...

- ... the GCC that came on my Fedora 40 system: 14.1.1 20240701
      (Red Hat 14.1.1-7)
- ... GCC built from the release tarball: 14.1.0 (GCC)
- ... GCC built from the current git trunk (commit 679086172b84be):
      15.0.0 20240724

All these versions show this behavior.

The bug however does *not* appear in gcc 13.3.1 20240522 (Red Hat 13.3.1-1) on.

-------------------------

Here are the gcc -v outputs:

14.1.1 installed on my system (Fedora 40):

    Using built-in specs.
    COLLECT_GCC=/usr/bin/gcc
    COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/14/lto-wrapper
    OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
    OFFLOAD_TARGET_DEFAULT=1
    Target: x86_64-redhat-linux
    Configured with: ../configure --enable-bootstrap
--enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,m2,lto --prefix=/usr
--mandir=/usr/share/man --infodir=/usr/share/info
--with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared
--enable-threads=posix --enable-checking=release --enable-multilib
--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
--enable-gnu-unique-object --enable-linker-build-id
--with-gcc-major-version-only --enable-libstdcxx-backtrace
--with-libstdcxx-zoneinfo=/usr/share/zoneinfo --with-linker-hash-style=gnu
--enable-plugin --enable-initfini-array
--with-isl=/builddir/build/BUILD/gcc-14.1.1-20240701/obj-x86_64-redhat-linux/isl-install
--enable-offload-targets=nvptx-none,amdgcn-amdhsa --enable-offload-defaulted
--without-cuda-driver --enable-gnu-indirect-function --enable-cet
--with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
--with-build-config=bootstrap-lto --enable-link-serialization=1
    Thread model: posix
    Supported LTO compression algorithms: zlib zstd
    gcc version 14.1.1 20240701 (Red Hat 14.1.1-7) (GCC)

GCC 14.1.0 built from source

    Using built-in specs.
    COLLECT_GCC=/home/laria/local/gcc/usr/local/bin/gcc
   
COLLECT_LTO_WRAPPER=/home/laria/local/gcc/usr/local/bin/../libexec/gcc/x86_64-pc-linux-gnu/14.1.0/lto-wrapper
    Target: x86_64-pc-linux-gnu
    Configured with: ./configure --disable-multilib
    Thread model: posix
    Supported LTO compression algorithms: zlib zstd
    gcc version 14.1.0 (GCC)

Current (well, yesterday's :) ) git trunk
(679086172b84be18c55fdbb9cda7e97806e7c083)

    Using built-in specs.
    COLLECT_GCC=/home/laria/src/gcc/build/install/usr/local/bin/gcc
   
COLLECT_LTO_WRAPPER=/home/laria/src/gcc/build/install/usr/local/bin/../libexec/gcc/x86_64-pc-linux-gnu/15.0.0/lto-wrapper
    Target: x86_64-pc-linux-gnu
    Configured with: ../configure --disable-multilib --enable-languages=c
    Thread model: posix
    Supported LTO compression algorithms: zlib zstd
    gcc version 15.0.0 20240724 (experimental) (GCC)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
@ 2024-07-25 21:46 ` pinskia at gcc dot gnu.org
  2024-07-25 21:50 ` sjames at gcc dot gnu.org
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-07-25 21:46 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
            Summary|_Bool value from tagged     |[14/15 Regression] _Bool
                   |union is incorrect when     |value from tagged union is
                   |built with -O1              |incorrect when built with
                   |                            |optimization
           Keywords|                            |wrong-code
   Target Milestone|---                         |14.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
  2024-07-25 21:46 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization pinskia at gcc dot gnu.org
@ 2024-07-25 21:50 ` sjames at gcc dot gnu.org
  2024-07-25 21:56 ` pinskia at gcc dot gnu.org
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sjames at gcc dot gnu.org @ 2024-07-25 21:50 UTC (permalink / raw)
  To: gcc-bugs

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

Sam James <sjames at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sjames at gcc dot gnu.org

--- Comment #1 from Sam James <sjames at gcc dot gnu.org> ---
Bisecting.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
  2024-07-25 21:46 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization pinskia at gcc dot gnu.org
  2024-07-25 21:50 ` sjames at gcc dot gnu.org
@ 2024-07-25 21:56 ` pinskia at gcc dot gnu.org
  2024-07-26  3:50 ` pinskia at gcc dot gnu.org
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-07-25 21:56 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-07-25
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed. introduced by my r14-6420-g85c5efcffed19c .

We go from:
  <bb 4> [local count: 1073741824]:
  if (value_7 == 0)
    goto <bb 5>; [50.00%]
  else
    goto <bb 6>; [50.00%]

  <bb 5> [local count: 536870912]:
<L2>:
  _6 = VIEW_CONVERT_EXPR<_Bool>(value$8_8);

  <bb 6> [local count: 1073741824]:
  # _2 = PHI <_6(5), 1(4)>

to:

  _6 = VIEW_CONVERT_EXPR<_Bool>(value$8_8);
  _12 = value_7 != 0;
  _13 = _6 | _12;

The VCE comes from SRA.

Now there is a few other issues here too.

First off If I change the switch to:
```
    if (value.type == VALUE_BOOLEAN)
      return value.boolean;
    return 1;
```

Then phiopt will move the VCE out of the if and then things will work; I should
try to understand what the difference there is.

Anyways this is for tomorrow or the weekend to solve. It won't be solved for
GCC 14.2.0 as that is already in RC1.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (2 preceding siblings ...)
  2024-07-25 21:56 ` pinskia at gcc dot gnu.org
@ 2024-07-26  3:50 ` pinskia at gcc dot gnu.org
  2024-07-26  4:31 ` pinskia at gcc dot gnu.org
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-07-26  3:50 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I will fix this. I will declare VCE that change into non-full precision mode
types as non-moveable. movement_possibility_1 in LIM should get a similar
change even though I have not seen wrong code due to that yet; I suspect there
has not been a checker for undefinedness yet for that.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (3 preceding siblings ...)
  2024-07-26  3:50 ` pinskia at gcc dot gnu.org
@ 2024-07-26  4:31 ` pinskia at gcc dot gnu.org
  2024-07-26  6:04 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-07-26  4:31 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So the check for VCE should be:
type_strictly_matches_mode_p (TREE_TYPE (lhs))

I am trying to figure out if the padding bits on real types matter here. I
don't think so since those padding are ignored always. while integer (and maybe
vector types with non-vector modes) do make a difference.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (4 preceding siblings ...)
  2024-07-26  4:31 ` pinskia at gcc dot gnu.org
@ 2024-07-26  6:04 ` rguenth at gcc dot gnu.org
  2024-07-26  6:07 ` pinskia at gcc dot gnu.org
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-26  6:04 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Why should they be non-moveable?  Because the padding might be not initialized?
That there's a V_C_E to bool looks odd (I assume the value type is void * or
int?)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (5 preceding siblings ...)
  2024-07-26  6:04 ` rguenth at gcc dot gnu.org
@ 2024-07-26  6:07 ` pinskia at gcc dot gnu.org
  2024-07-26  6:08 ` rguenth at gcc dot gnu.org
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-07-26  6:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #5)
> Why should they be non-moveable?  Because the padding might be not
> initialized?
> That there's a V_C_E to bool looks odd (I assume the value type is void * or
> int?)
The value type is unsigned char.
The VCE comes from SRA, see pr 99919 for reports of it before.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (6 preceding siblings ...)
  2024-07-26  6:07 ` pinskia at gcc dot gnu.org
@ 2024-07-26  6:08 ` rguenth at gcc dot gnu.org
  2024-07-27  0:38 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43 pinskia at gcc dot gnu.org
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-26  6:08 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ah, it's the code that makes SRA widen bit-precision values to size-precision
integers.  That said, I don't see why the V_C_E should not be moveable.

  value$8_11 = MEM <unsigned char> [(struct Value *)&s_value + 8B];
  _8 = VIEW_CONVERT_EXPR<_Bool>(value$8_11);

is exactly what RTL expansion will create - we're not truncating the
result to bit-precision on _Bool loads.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (7 preceding siblings ...)
  2024-07-26  6:08 ` rguenth at gcc dot gnu.org
@ 2024-07-27  0:38 ` pinskia at gcc dot gnu.org
  2024-07-27 11:01 ` rguenther at suse dot de
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-07-27  0:38 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Ah, it's the code that makes SRA widen bit-precision values to
> size-precision integers.  That said, I don't see why the V_C_E should not be
> moveable.
> 
>   value$8_11 = MEM <unsigned char> [(struct Value *)&s_value + 8B];
>   _8 = VIEW_CONVERT_EXPR<_Bool>(value$8_11);
> 
> is exactly what RTL expansion will create - we're not truncating the
> result to bit-precision on _Bool loads.

The problem is the bits outside of bool are set and we don't truncate them out
and since we turn it into:

  _6 = VIEW_CONVERT_EXPR<_Bool>(value$8_8);
  _12 = value_7 != 0;
  _13 = _6 | _12;

We get the wrong answer. This is very similar to if _6 had been an
uninitialized which we fixed during the development of GCC 14.
That is the range of _6 (or _8) is only conditionally `[0,1]`, otherwise it
should be considered similar to an uninitialized variable.

The other thing which we might be able to solve this is change:
```
/* For integral conversions with the same precision or pointer
   conversions use a NOP_EXPR instead.  */
(simplify
  (view_convert @0)
  (if ((INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type))
       && (INTEGRAL_TYPE_P (TREE_TYPE (@0)) || POINTER_TYPE_P (TREE_TYPE (@0)))
       && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)))
   (convert @0)))
```

to change bool convert to be convert too. In a similar way how we handle
`zero_one != 0` into a convert instead of a VCE (in VPR and other passes). This
will fix the above too.

Maybe I will test that to see what code generation will look like.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (8 preceding siblings ...)
  2024-07-27  0:38 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43 pinskia at gcc dot gnu.org
@ 2024-07-27 11:01 ` rguenther at suse dot de
  2024-08-01  9:41 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenther at suse dot de @ 2024-07-27 11:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> ---
> Am 27.07.2024 um 02:38 schrieb pinskia at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116098
> 
> --- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #7)
>> Ah, it's the code that makes SRA widen bit-precision values to
>> size-precision integers.  That said, I don't see why the V_C_E should not be
>> moveable.
>> 
>>  value$8_11 = MEM <unsigned char> [(struct Value *)&s_value + 8B];
>>  _8 = VIEW_CONVERT_EXPR<_Bool>(value$8_11);
>> 
>> is exactly what RTL expansion will create - we're not truncating the
>> result to bit-precision on _Bool loads.
> 
> The problem is the bits outside of bool are set and we don't truncate them out
> and since we turn it into:
> 
>  _6 = VIEW_CONVERT_EXPR<_Bool>(value$8_8);
>  _12 = value_7 != 0;
>  _13 = _6 | _12;
> 
> We get the wrong answer. This is very similar to if _6 had been an
> uninitialized which we fixed during the development of GCC 14.
> That is the range of _6 (or _8) is only conditionally `[0,1]`, otherwise it
> should be considered similar to an uninitialized variable.
> 
> The other thing which we might be able to solve this is change:
> ```
> /* For integral conversions with the same precision or pointer
>   conversions use a NOP_EXPR instead.  */
> (simplify
>  (view_convert @0)
>  (if ((INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type))
>       && (INTEGRAL_TYPE_P (TREE_TYPE (@0)) || POINTER_TYPE_P (TREE_TYPE (@0)))
>       && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)))
>   (convert @0)))
> ```
> 
> to change bool convert to be convert too. In a similar way how we handle
> `zero_one != 0` into a convert instead of a VCE (in VPR and other passes). This
> will fix the above too.
> 
> Maybe I will test that to see what code generation will look like.

That’s a possibility though it will affect all uses and cause a lot of extra
truncation I fear.

Richard 

> --
> You are receiving this mail because:
> You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (9 preceding siblings ...)
  2024-07-27 11:01 ` rguenther at suse dot de
@ 2024-08-01  9:41 ` jakub at gcc dot gnu.org
  2024-08-30 16:28 ` pinskia at gcc dot gnu.org
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2024-08-01  9:41 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|14.2                        |14.3

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 14.2 is being released, retargeting bugs to GCC 14.3.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (10 preceding siblings ...)
  2024-08-01  9:41 ` jakub at gcc dot gnu.org
@ 2024-08-30 16:28 ` pinskia at gcc dot gnu.org
  2024-08-30 21:54 ` pinskia at gcc dot gnu.org
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-08-30 16:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 59028
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=59028&action=edit
semi-cleaned up testcase

For this testcase you need to compile with -DBROKEN=1 to see the broken code.
It becomes obvious that the difference is the label when it comes to phiopt
here. The heurstics that was added for PR71016 don't ignore labels (and other
nop related things). Fixing that fixes the wrong code and might improve other
cases too.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (11 preceding siblings ...)
  2024-08-30 16:28 ` pinskia at gcc dot gnu.org
@ 2024-08-30 21:54 ` pinskia at gcc dot gnu.org
  2024-08-31 16:17 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-08-30 21:54 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 59029
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=59029&action=edit
Simplified C++ testcase

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (12 preceding siblings ...)
  2024-08-30 21:54 ` pinskia at gcc dot gnu.org
@ 2024-08-31 16:17 ` cvs-commit at gcc dot gnu.org
  2024-08-31 16:18 ` [Bug tree-optimization/116098] [14 " pinskia at gcc dot gnu.org
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-08-31 16:17 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:ceda727dafba6e05b510b5f8f4ccacfb507dc023

commit r15-3334-gceda727dafba6e05b510b5f8f4ccacfb507dc023
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Fri Aug 30 10:36:24 2024 -0700

    phiopt: Ignore some nop statements in heursics [PR116098]

    The heurstics that was added for PR71016, try to search to see
    if the conversion was being moved away from its definition. The problem
    is the heurstics would stop if there was a non GIMPLE_ASSIGN (and already
ignores
    debug statements) and in this case we would have a GIMPLE_LABEL that was
not
    being ignored. So we should need to ignore GIMPLE_NOP, GIMPLE_LABEL and
GIMPLE_PREDICT.
    Note this is now similar to how gimple_empty_block_p behaves.

    Note this fixes the wrong code that was reported by moving the VCE
(conversion) out before
    the phiopt/match could convert it into an bit_ior and move the VCE out with
the VCE being
    conditionally valid.

    Bootstrapped and tested on x86_64-linux-gnu.
    Also built and tested for aarch64-linux-gnu.

            PR tree-optimization/116098

    gcc/ChangeLog:

            * tree-ssa-phiopt.cc (factor_out_conditional_operation): Ignore
            nops, labels and predicts for heuristic for conversion with a
constant.

    gcc/testsuite/ChangeLog:

            * c-c++-common/torture/pr116098-1.c: New test.
            * gcc.target/aarch64/csel-1.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (13 preceding siblings ...)
  2024-08-31 16:17 ` cvs-commit at gcc dot gnu.org
@ 2024-08-31 16:18 ` pinskia at gcc dot gnu.org
  2024-09-06 19:48 ` laria at laria dot me
  2024-09-06 19:56 ` [Bug tree-optimization/116098] [14/15 " pinskia at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-08-31 16:18 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |14.1.0, 14.2.0
            Summary|[14/15 Regression] _Bool    |[14 Regression] _Bool value
                   |value from tagged union is  |from tagged union is
                   |incorrect when built with   |incorrect when built with
                   |optimization since          |optimization since
                   |r14-1597-g64d90d06d2db43    |r14-1597-g64d90d06d2db43
      Known to work|                            |13.1.0, 13.3.0, 15.0

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed so far on the trunk.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (14 preceding siblings ...)
  2024-08-31 16:18 ` [Bug tree-optimization/116098] [14 " pinskia at gcc dot gnu.org
@ 2024-09-06 19:48 ` laria at laria dot me
  2024-09-06 19:56 ` [Bug tree-optimization/116098] [14/15 " pinskia at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: laria at laria dot me @ 2024-09-06 19:48 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Laria Chabowski <laria at laria dot me> ---
(In reply to Andrew Pinski from comment #14)
> Fixed so far on the trunk.

Thank you! That fixed the test cases I provided.

However, when I tested it against the code where I stumbled upon this, I still
got an unexpected result.

Essentially the same idea: A function truthy() that switches over the type
enum,
returns the bool value if it's bool or always false if the type is nil or true
in all other cases. Then, inverting it and saving and reading it as a value
struct again, some garbage comes out and when using the resulting bool in a
ternary, the opposite of the expected happens.

I've reduced that code to this:

======= BEGIN test.c =======
int puts(const char *);

struct Value {
    enum value_type {
        VALUE_NIL,
        VALUE_BOOLEAN,
        VALUE_MAGIC,
    } type;
    union {
        _Bool boolean;
        void *p[2];
    };
};

static struct Value s_item_mem;

static void
set_bool(_Bool b)
{
    s_item_mem = (struct Value) {
        .type = VALUE_BOOLEAN,
        .boolean = b,
    };
}

static void
set_magic(void)
{
    s_item_mem = (struct Value) {
        .type = VALUE_MAGIC,
        .p[0] = (void *)0x7fff0123456789a0, // just something that vaguely
looks
                                            // like a pointer. In the original,
                                            // this was a valid pointer
returned
                                            // from malloc. But since this
                                            // reduced code never reads from
                                            // here it shouldn't matter that
                                            // it's not an actual address.
    };
}

static struct Value
val_get(void)
{
    // returning s_item_mem immediately makes the bug go away
    struct Value value = s_item_mem;
    return value;
}

static _Bool
truthy(void)
{
    // Inlinig val_get makes the bug go away
    struct Value value = val_get();
    switch (value.type) {
    case VALUE_NIL: // Removing this case makes the bug go away
        return 0;
    case VALUE_BOOLEAN:
        return value.boolean;
    default:
        return 1;
    }
}

int
main(void)
{
    set_magic(); // sets type to VALUE_MAGIC
    set_bool(!truthy()); // truthy should take default case, so
                         // set_bool(!1) -> set_bool(0)

    _Bool b = truthy(); // Should be 0 now
    puts(b ? "true" : "false"); // Should print false, prints true
    return b ? 0 : 1; // Exit code should be 1, is 161
}
======= END test.c =======

I built it with:

gcc -Wall -Werror -Wextra -pedantic -O2 -fno-strict-aliasing -fwrapv \
    -fno-aggressive-loop-optimizations test.c

Happens both with the GCC installed on my Fedora 40 machine:

    $ gcc -v
    Using built-in specs.
    COLLECT_GCC=/usr/bin/gcc
    COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/14/lto-wrapper
    OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
    OFFLOAD_TARGET_DEFAULT=1
    Target: x86_64-redhat-linux
    Configured with: ../configure --enable-bootstrap
--enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,m2,lto --prefix=/usr
--mandir=/usr/share/man --infodir=/usr/share/info
--with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared
--enable-threads=posix --enable-checking=release --enable-multilib
--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
--enable-gnu-unique-object --enable-linker-build-id
--with-gcc-major-version-only --enable-libstdcxx-backtrace
--with-libstdcxx-zoneinfo=/usr/share/zoneinfo --with-linker-hash-style=gnu
--enable-plugin --enable-initfini-array
--with-isl=/builddir/build/BUILD/gcc-14.2.1-20240801/obj-x86_64-redhat-linux/isl-install
--enable-offload-targets=nvptx-none,amdgcn-amdhsa --enable-offload-defaulted
--without-cuda-driver --enable-gnu-indirect-function --enable-cet
--with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
--with-build-config=bootstrap-lto --enable-link-serialization=1
    Thread model: posix
    Supported LTO compression algorithms: zlib zstd
    gcc version 14.2.1 20240801 (Red Hat 14.2.1-1) (GCC)

And with the one I built a couple of days ago from the git trunk, that includes
the fix (HEAD was at ce5f2dc45038c9806088132cc923b13719f48732 when I built it.
git log from here includes the commit ceda727dafba6 with the fix):

    $ ~/local/gcc/usr/local/bin/x86_64-linux-gcc -v
    Using built-in specs.
    COLLECT_GCC=/home/laria/local/gcc/usr/local/bin/x86_64-linux-gcc
   
COLLECT_LTO_WRAPPER=/home/laria/local/gcc/usr/local/bin/../libexec/gcc/x86_64-linux/15.0.0/lto-wrapper
    Target: x86_64-linux
    Configured with: ../configure --target=x86_64-linux --disable-multilib
    Thread model: posix
    Supported LTO compression algorithms: zlib zstd
    gcc version 15.0.0 20240903 (experimental) (GCC)

Also reproducible on godbolt.org, which apparently uses this version for trunk:

    g++
(Compiler-Explorer-Build-gcc-1735917cee41fe680d9dd3c0c26b45520c17413a-binutils-2.42)
15.0.0 20240906 (experimental)

Please let me know if I can provide any more info.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43
  2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
                   ` (15 preceding siblings ...)
  2024-09-06 19:48 ` laria at laria dot me
@ 2024-09-06 19:56 ` pinskia at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-09-06 19:56 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[14 Regression] _Bool value |[14/15 Regression] _Bool
                   |from tagged union is        |value from tagged union is
                   |incorrect when built with   |incorrect when built with
                   |optimization since          |optimization since
                   |r14-1597-g64d90d06d2db43    |r14-1597-g64d90d06d2db43

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Laria Chabowski from comment #15)
> (In reply to Andrew Pinski from comment #14)
> > Fixed so far on the trunk.
> 
> Thank you! That fixed the test cases I provided.
> 
> However, when I tested it against the code where I stumbled upon this, I
> still
> got an unexpected result.
> 
> Essentially the same idea: A function truthy() that switches over the type
> enum,
> returns the bool value if it's bool or always false if the type is nil or
> true
> in all other cases. Then, inverting it and saving and reading it as a value
> struct again, some garbage comes out and when using the resulting bool in a
> ternary, the opposite of the expected happens.

Yes it is the same bug just I had worked around the previous one really :).

Here we have:
```
  if (value_10 == 0)
    goto <bb 5>; [33.33%]
  else
    goto <bb 3>; [66.67%]

  <bb 3> [local count: 715827875]:
  if (value_10 == 1)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 357913942]:
<L1>:
  _4 = VIEW_CONVERT_EXPR<_Bool>(value$8_11);

  <bb 5> [local count: 1073741824]:
  # _1 = PHI <0(2), _4(4), 1(3)>
```

And we are optimizing bb3/bb4/bb5 and in this case we can't move out the VCE of
the condition since it is from 3 bb rather than just 2. 

Back to drawing board.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-09-06 19:56 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-25 21:21 [Bug c/116098] New: _Bool value from tagged union is incorrect when built with -O1 laria at laria dot me
2024-07-25 21:46 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization pinskia at gcc dot gnu.org
2024-07-25 21:50 ` sjames at gcc dot gnu.org
2024-07-25 21:56 ` pinskia at gcc dot gnu.org
2024-07-26  3:50 ` pinskia at gcc dot gnu.org
2024-07-26  4:31 ` pinskia at gcc dot gnu.org
2024-07-26  6:04 ` rguenth at gcc dot gnu.org
2024-07-26  6:07 ` pinskia at gcc dot gnu.org
2024-07-26  6:08 ` rguenth at gcc dot gnu.org
2024-07-27  0:38 ` [Bug tree-optimization/116098] [14/15 Regression] _Bool value from tagged union is incorrect when built with optimization since r14-1597-g64d90d06d2db43 pinskia at gcc dot gnu.org
2024-07-27 11:01 ` rguenther at suse dot de
2024-08-01  9:41 ` jakub at gcc dot gnu.org
2024-08-30 16:28 ` pinskia at gcc dot gnu.org
2024-08-30 21:54 ` pinskia at gcc dot gnu.org
2024-08-31 16:17 ` cvs-commit at gcc dot gnu.org
2024-08-31 16:18 ` [Bug tree-optimization/116098] [14 " pinskia at gcc dot gnu.org
2024-09-06 19:48 ` laria at laria dot me
2024-09-06 19:56 ` [Bug tree-optimization/116098] [14/15 " pinskia at gcc dot gnu.org

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).