[Bug libstdc++/113376] New: Confusing notes when using C++17 parallel algorithms

[Bug libstdc++/113376] New: Confusing notes when using C++17 parallel algorithms

[pilarlatiesa at gmail dot com]

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

            Bug ID: 113376
           Summary: Confusing notes when using C++17 parallel algorithms
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pilarlatiesa at gmail dot com
  Target Milestone: ---

With GCC 14, the following code:

#include <algorithm>
#include <execution>
#include <vector>

void f(std::vector<int> &v)
{
std::for_each(std::execution::par, v.begin(), v.end(),
              [](int &i) { i *= 2; });
}

when compiled emits a lot of notes like:

/home/pililatiesa/gcc-14/include/c++/14.0.0/pstl/algorithm_impl.h: In function
'_RandomAccessIterator
__pstl::__internal::__brick_unique(_RandomAccessIterator,
_RandomAccessIterator, _BinaryPredicate, std::true_type)':
/home/pililatiesa/gcc-14/include/c++/14.0.0/pstl/algorithm_impl.h:1219:5: note:
'#pragma message:  [Parallel STL message]: "Vectorized algorithm unimplemented,
redirected to serial"'
 1219 |     _PSTL_PRAGMA_MESSAGE("Vectorized algorithm unimplemented,
redirected to serial");
      |     ^~~~~~~~~~~~~~~~~~~~

I don't understand why all these functions are even instantiated as they appear
to be related to the vectorization of other algorithms.

Furthermore, in pstl_config.h we have:

// Check the user-defined macro for warnings
#if defined(PSTL_USAGE_WARNINGS)
#    undef _PSTL_USAGE_WARNINGS
#    define _PSTL_USAGE_WARNINGS PSTL_USAGE_WARNINGS
// Check the internal macro for warnings
#elif !defined(_PSTL_USAGE_WARNINGS)
#    define _PSTL_USAGE_WARNINGS 0
#endif

and later in this file:


#if defined(_PSTL_USAGE_WARNINGS)
#    define _PSTL_PRAGMA_MESSAGE(x) _PSTL_PRAGMA_MESSAGE_IMPL(x)
#    define _PSTL_PRAGMA_MESSAGE_POLICIES(x) _PSTL_PRAGMA_MESSAGE_IMPL(x)
#else
#    define _PSTL_PRAGMA_MESSAGE(x)
#    define _PSTL_PRAGMA_MESSAGE_POLICIES(x)
#endif

i.e. is checking defined(_PSTL_USAGE_WARNINGS) instead of just
_PSTL_USAGE_WARNINGS. This logic doesn't seem right.

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-01-13
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
                 CC|                            |rodgertq at gcc dot gnu.org

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[pilarlatiesa at gmail dot com]

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

--- Comment #1 from Pilar Latiesa <pilarlatiesa at gmail dot com> ---
(In reply to Pilar Latiesa from comment #0)
> I don't understand why all these functions are even instantiated as they
> appear to be related to the vectorization of other algorithms.

Pragma messages are shown irrespectively of whether the function is
instantiated.

> i.e. is checking defined(_PSTL_USAGE_WARNINGS) instead of just
> _PSTL_USAGE_WARNINGS. This logic doesn't seem right.

This change was made as part of r14-2109-g3162ca09dbdc2e:

- #if _PSTL_USAGE_WARNINGS
+ #if defined(_PSTL_USAGE_WARNINGS)

I don’t think this change is correct.

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Pilar Latiesa from comment #1)
> (In reply to Pilar Latiesa from comment #0)
> > I don't understand why all these functions are even instantiated as they
> > appear to be related to the vectorization of other algorithms.
> 
> Pragma messages are shown irrespectively of whether the function is
> instantiated.

Yeah, I don't know if Clang works differently (I don't think so?) or if those
pragmas are just misused. They come from the upstream PSTL project.

> 
> > i.e. is checking defined(_PSTL_USAGE_WARNINGS) instead of just
> > _PSTL_USAGE_WARNINGS. This logic doesn't seem right.
> 
> This change was made as part of r14-2109-g3162ca09dbdc2e:
> 
> - #if _PSTL_USAGE_WARNINGS
> + #if defined(_PSTL_USAGE_WARNINGS)
> 
> I don’t think this change is correct.

Good catch.

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[pilarlatiesa at gmail dot com]

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

--- Comment #3 from Pilar Latiesa <pilarlatiesa at gmail dot com> ---

It seems that what is missing is a corresponding change in the macro definition
logic. It should have been changed to:

// Check the user-defined macro for warnings
#if defined(PSTL_USAGE_WARNINGS)
#    define _PSTL_USAGE_WARNINGS
#endif

See
https://github.com/llvm/llvm-project/blob/5ccf19ded09f68bef43275c81c20b0e65f7c0b75/pstl/include/pstl/internal/pstl_config.h#L26

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
https://github.com/llvm/llvm-project/commit/c4823cc5db69f16bb5c96cf7d1b0d070da83605e
changed the logic upstream, but it looks like that didn't come downstream with
r14-2109-g3162ca09dbdc2e

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think
https://github.com/llvm/llvm-project/commit/3b9a1bb1af90db9472340ef2122d3855eb9ba3fc#r142768040
is the real cause of the problem. They wanted to avoid -Wundef errors, so
changed all the macro tests to use #ifdef instead of #if

That is what requires a change to how _PSTL_USAGE_WARNINGS is defined. It also
caused other problems elsewhere downstream:
https://github.com/oneapi-src/oneDPL/issues/1602

[Bug libstdc++/113376] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Pilar Latiesa from comment #3)
> It seems that what is missing is a corresponding change in the macro
> definition logic. It should have been changed to:
> 
> // Check the user-defined macro for warnings
> #if defined(PSTL_USAGE_WARNINGS)
> #    define _PSTL_USAGE_WARNINGS
> #endif
> 
> See
> https://github.com/llvm/llvm-project/blob/
> 5ccf19ded09f68bef43275c81c20b0e65f7c0b75/pstl/include/pstl/internal/
> pstl_config.h#L26

The upstream change breaks the previous API though.

It looks like users were able to define PSTL_USAGE_WARNINGS=0 or
PSTL_USAGE_WARNINGS=1 to request warnings to be off or on, respectively. But
the LLVM change means that it only matters whether it's defined or not. This
seems to have been a misguided change to make sure *all* macros are used with
#ifdef not #if. But that breaks the intended use for some of them.

So I think we should either revert the change later in the file which checks
_PSTL_USAGE_WARNINGS, or we should take the user-facing macro's value into
account when deciding whether to define it:

// Check the user-defined macro for warnings
#if defined(PSTL_USAGE_WARNINGS) && PSTL_USAGE_WARNINGS != 0
#    define _PSTL_USAGE_WARNINGS
#endif

[Bug libstdc++/113376] [14/15 Regression] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |14.1.0, 15.0
      Known to work|                            |13.3.0
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Target Milestone|---                         |14.2
            Summary|Confusing notes when using  |[14/15 Regression]
                   |C++17 parallel algorithms   |Confusing notes when using
                   |                            |C++17 parallel algorithms

[Bug libstdc++/113376] [14/15 Regression] Confusing notes when using C++17 parallel algorithms

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                URL|                            |https://gcc.gnu.org/piperma
                   |                            |il/gcc-patches/2024-June/65
                   |                            |4411.html
           Keywords|                            |patch

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Patch posted: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654411.html

[Bug libstdc++/113376] [14/15 Regression] Confusing notes when using C++17 parallel algorithms

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:99a1fe6c12c733fe4923a75a79d09a66ff8abcec

commit r15-1255-g99a1fe6c12c733fe4923a75a79d09a66ff8abcec
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jun 12 16:47:17 2024 +0100

    libstdc++: Fix unwanted #pragma messages from PSTL headers [PR113376]

    When we rebased the PSTL on upstream, in r14-2109-g3162ca09dbdc2e, a
    change to how _PSTL_USAGE_WARNINGS is set was missed out, but the change
    to how it's tested was included. This means that the macro is always
    defined, so testing it with #ifdef (instead of using #if to test its
    value) doesn't work as intended.

    Revert the test to use #if again, since that part of the upstream change
    was unnecessary in the first place (the macro is always defined, so
    there's no need to use #ifdef to avoid -Wundef warnings).

    libstdc++-v3/ChangeLog:

            PR libstdc++/113376
            * include/pstl/pstl_config.h: Use #if instead of #ifdef to test
            the _PSTL_USAGE_WARNINGS macro.