Enable CTAD by default for GCC 9

[mhoemmen]

The implementation currently disables CTAD by default for GCC < 11.

https://github.com/youyu3/mdspan/blob/d4ba2621d097d31c85399f16b2976b3643bce08e/include/experimental/__p0009_bits/config.hpp#L220

However, it is known to work for GCC 9.2.0. @youyu3 confirmed that

• CTAD works with both nvcc + GCC 9.2.0, and GCC 9.2.0 alone; and
• CTAD does not work with GCC 10.2.0.

Thus, we should consider enabling CTAD by default for GCC 9, and only disabling it by default for GCC < 9 or GCC 10.

[youyu3]

To clarify, I only tested with https://github.com/kokkos/mdspan/blob/stable/examples/godbolt_starter/godbolt_starter.cpp.

[crtrott]

Did you test all other GCC 9.x compilers? I.e. do they work all?

[youyu3]

My machine only has GCC 9.1, 9.2 and 9.3. It worked for all of them. I'll need to install GCC 9.4 and 9.5 to test them.

[adah1972]

I do not know what made Bryce @brycelelbach say "GCC 10's CTAD seems sufficiently broken to prevent its use" in commit 972a4bc, but currently CTAD seems to work well in all C++17-supporting GCC versions. The following command works well:

g++ -std=c++17 -D_MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION \
    -Iinclude examples/godbolt_starter/godbolt_starter.cpp

I tested with the following GCC versions:

• 7.50
• 8.4.0
• 9.4.0
• 10.3.0
• 10.4.0
• 11.3.0
• 12.2.0

The comment in commit 2141453 mentioned a GCC bug 100983, which was fixed only in GCC 12. However, the current mdspan code does not seem to rely on the absence of this bug.

So I would suggest removing the (defined(_MDSPAN_COMPILER_CLANG) || !defined(__GNUC__) || __GNUC__ >= 11) conditionals, unless we have new reasons to believe otherwise.

[brycelelbach]

I think whatever we were doing at the time didn't work with GCC 10.

…

On Thu, Feb 9, 2023 at 5:13 AM Wu Yongwei ***@***.***> wrote: I do not know what made Bryce @brycelelbach <https://github.com/brycelelbach> say "GCC 10's CTAD seems sufficiently broken to prevent its use" in commit 972a4bc <972a4bc>, but currently CTAD seems to work well in all C++17-supporting GCC versions. The following command works well: g++ -std=c++17 -D_MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION \ -Iinclude examples/godbolt_starter/godbolt_starter.cpp I tested with the following GCC versions: - 7.50 - 8.4.0 - 9.4.0 - 10.3.0 - 10.4.0 - 11.3.0 - 12.2.0 The comment in commit 2141453 <2141453> mentioned a GCC bug 100983 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100983>, which was fixed only in GCC 12. However, the current mdspan code does not seem to rely on the absence of this bug. So I would suggest removing the (defined(_MDSPAN_COMPILER_CLANG) || !defined(__GNUC__) || __GNUC__ >= 11) conditionals, unless we have new reasons to believe otherwise. — Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBG4XDHZ34OG2ZVS5BORTWWTUPDANCNFSM6AAAAAATBLBKNQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Bryce Adelstein Lelbach aka wash (he/him/his) US Programming Language Standards Chair ISO C++ Library Evolution Chair Principal Architect @ NVIDIA --

[adah1972]

@brycelelbach

Hi Bryce, I believe there was a reason, but I do not know (as the current example does not even work with clang at commit 2141453). The key question is whether the reason still holds.

I believe not.

Do you think I should submit a patch to remove all the GCC conditionals?

[brycelelbach]

Yes

…

-- Bryce Adelstein Lelbach aka wash (he/him/his) US Programming Language Standards Chair ISO C++ Library Evolution Chair Principal Architect @ NVIDIA --

On Fri, Feb 10, 2023, 18:46 Wu Yongwei ***@***.***> wrote: @brycelelbach <https://github.com/brycelelbach> Hi Bryce, I believe there was a reason, but I do not know (as the current example does not even work with clang at commit 2141453 <2141453>). The key question is whether the reason still holds. I believe not. Do you think I should submit a patch to remove all the GCC conditionals? — Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBG4RJJN5SX5RD2WA6OBLWW34RNANCNFSM6AAAAAATBLBKNQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[adah1972]

PR submitted. Please review.

@brycelelbach

[brycelelbach]

I think it'd be best for Christian Trott or one of the current maintainers to review. It seems fine to me but I'm not as familiar with the codebase.

…

On Sat, Feb 11, 2023 at 5:11 AM Wu Yongwei ***@***.***> wrote: PR submitted. Please review. @brycelelbach <https://github.com/brycelelbach> — Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBG4VEKSTHQQ22DSGKHFTWW6FYZANCNFSM6AAAAAATBLBKNQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Bryce Adelstein Lelbach aka wash (he/him/his) US Programming Language Standards Chair ISO C++ Library Evolution Chair Principal Architect @ NVIDIA --

[adah1972]

@crtrott Hi Chris, would you please take a look? It is a simple change.

[mhoemmen]

@adah1972 FYI, his name is Christian, not Chris. It's Sunday and he just finished a week-long WG21 meeting (in fact, we all just did), so he might not get to this immediately. Thank you for your contribution!

[adah1972]

Ha, looking up the dictionary, I've found that Chris is not the shortened form of Christian. Sorry for my mistake.

No hurry.