[Bug c++/111097] New: mistifying bug triggered by trivial code change

[Bug c++/111097] New: mistifying bug triggered by trivial code change

[fabio at cannizzo dot net]

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

            Bug ID: 111097
           Summary: mistifying bug triggered by trivial code change
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fabio at cannizzo dot net
  Target Milestone: ---

Created attachment 55774
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55774&action=edit
reproducible test case

I come across a weird bug. If I instrument the code with traces, the bug
disappears. If I remove some force inline attributes, the bug also disappears.

Eventually I manage to reduce to a minimalist example.

The attached file contains a class with a member state vector which is used as
a circular buffer
alignas(64) __m128i m_state[156];

The constructor fills it with some chosen numbers.

The first time the method `genrand_uint32` is called, it invokes the `refill`
function, which is supposed to modify all members of the `m_state` vector. Then
we extract one by one the element in the vector as `uint32_t`.

We print the 4 and last 4 elements of the state vector, reinterpreted as
`uint32_t[]` before refill and after refill, and the content is correct. We
also print the first and last 2 elements of the array extracted by
`genrand_uint32`, which should match exactly the state vector after refill, but
the first element is wrong.

 pre-refill: 3403902862 1263040418  ... 2214668603 3759392492 
post-refill: 1643499593 2393179749  ... 1152747345 4215414382 
  extracted: 3403902862 2393179749  ... 1152747345 4215414382

To be clear, the first element extracted should be 1643499593, i.e. the correct
output should be:

 pre-refill: 3403902862 1263040418  ... 2214668603 3759392492 
post-refill: 1643499593 2393179749  ... 1152747345 4215414382 
  extracted: 1643499593 2393179749  ... 1152747345 4215414382

If I remove a few of the FORCE_INLINE macros, e.g. if I change
   FORCE_INLINE void refill()
to
   NO_INLINE void refill()
the error disappears.

If I change 1 to 0 in this `#ifdef`, the error disappears.
   #if 1  // here p is of type __m128i* and tmp has type __m128i
       struct XV {
           __m128i m_v;
           XV(__m128i v) : m_v(v) {}
       };

       ((XV *)p)[0] = tmp;
   #else
       p[0] = tmp;
   #endif

If I reorder the `genrand_uint32` from
    if (m_prnd != end()) { // most likely case
        return *m_prnd++;
    }
    else {
        refill();
        m_prnd = begin();
        return *m_prnd++;
    }
to
    if (m_prnd == end()) {
        refill();
        m_prnd = begin();
    }
    return *m_prnd++;
the error disappears.

A godbolt link to this code is also available:
https://godbolt.org/z/v9b7z8c66

[Bug c++/111097] mistifying bug triggered by trivial code change

[fabio at cannizzo dot net]

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

--- Comment #1 from Fabio Cannizzo <fabio at cannizzo dot net> ---
Changing the `#if 1` to `#if 0` is probably the easiest way to isolate the
debug. It causes only minimal changes to the generated assembler.

I forgot to mention, the code is compiled with the flags
-O3 -msse4.2 -std=c++17

[Bug c++/111097] mistifying bug triggered by trivial code change

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Try -fno-strict-aliasing.  XV doesn't inter-operate
with uint32_t which you use in printSome to access the values stored via
an lvalue of type XV.

[Bug c++/111097] mistifying bug triggered by trivial code change

[fabio at cannizzo dot net]

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

--- Comment #3 from Fabio Cannizzo <fabio at cannizzo dot net> ---
Hi Richard

Using -fno-strict-aliasing the issue disappear, however I am not sure if that
is a real fix, or if it is merely circumstantial.

As mentioned, there are many other possible workarounds which also make the
issue disappear. The most curious one being the one below, where p has type
__m128i*, and simply casting it to XV* before the assignment, things stop
working.

Worth noting, this code works correctly with Visual Studio and with clang.


    template <int N, int JB>
    static FORCE_INLINE void doLoop(__m128i*& p, __m128i& xC, __m128i& xD,
__m128i bmask)
    {
        auto pend = p + N;
        do {
            __m128i tmp = advance1(p[0], p[JB], xC, xD, bmask);
            xC = xD;
            xD = tmp;
#if 1
            struct XV {
                __m128i m_v;
                XV(__m128i v) : m_v(v) {}
            };

            ((XV *)p)[0] = tmp;
#else
            p[0] = tmp;
#endif            
            ++p;
        } while (p != pend);
    }

[Bug c++/111097] mistifying bug triggered by trivial code change

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
You can mark XV as may_alias attribute to fix the code too.

[Bug c++/111097] mistifying bug triggered by trivial code change

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> Worth noting, this code works correctly with Visual Studio and with clang.

Gcc is known to be take more into account when it comes to aliasing rules of c
and c++. That is exactly what you are running into really.

[Bug c++/111097] mistifying bug triggered by trivial code change

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Fabio Cannizzo from comment #3)
> Using -fno-strict-aliasing the issue disappear, however I am not sure if
> that is a real fix, or if it is merely circumstantial.

When creating the bug report there was a notice at the top of the page:

"Before reporting that GCC compiles your code incorrectly, compile it with gcc
-Wall -Wextra and see whether this shows anything wrong with your code.
Similarly, if compiling with -fno-strict-aliasing -fwrapv makes a difference,
your code probably is not correct."

That is the case here.

> As mentioned, there are many other possible workarounds which also make the
> issue disappear. The most curious one being the one below, where p has type
> __m128i*, and simply casting it to XV* before the assignment, things stop
> working.

Yes, code with undefined behaviour is fragile and unpredictable.

> Worth noting, this code works correctly with Visual Studio and with clang.

Yes. code with undefined behaviour sometimes appears to work correctly with
different compilers, or just with different optimizations enabled.

[Bug c++/111097] mistifying bug triggered by trivial code change

[fabio at cannizzo dot net]

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

--- Comment #7 from Fabio Cannizzo <fabio at cannizzo dot net> ---
Apologies for my ignorance. Thank you.