kokkos / stdBLAS

Reference Implementation for stdBLAS
Other
118 stars 22 forks source link

Strange issue when compiling with MSVC 2022 #242

Closed stigrs closed 1 month ago

stigrs commented 2 years ago

I am trying to compile PR #241 using MSVC 2022 (version 19.32), but get the following C2653 error:

[build] C:\Users\srs\src\stdBLAS\include\experimental\__p1673_bits/transposed.hpp(255,57): error C2653: 'original_mapping_type': is not a class or namespace name [C:\Users\srs\src\stdBLAS\build\tests\native\givens.vcxproj]
[build] C:\Users\srs\src\stdBLAS\include\experimental\__p1673_bits/transposed.hpp(253,17): message : This diagnostic occurred in the compiler generated function 'auto std::experimental::__p1673_version_0::linalg::impl::transposed_layout<std::experimental::layout_stride>::mapping(const std::experimental::layout_stride::mapping<Extents> &)' [C:\Users\srs\src\stdBLAS\build\tests\native\givens.vcxproj]

When I use clang++ provided with MSVC 2022, there are no such errors.

mhoemmen commented 2 years ago

Thanks for reporting! @youyu3 reported a different error with a different compiler (that layout_stride::mapping has no rank() member function). We will investigate.

mhoemmen commented 2 years ago

@stigrs I pushed a commit that fixes a possibly related build issue with GCC 9.4.0. Could you please pull the branch and try again? Thanks!

stigrs commented 2 years ago

Hi. The fix for GCC 9.4.0 did not work unfortunately. I made the following ugly hack to lines 254-255 in transposed.hpp which works with MSVC 2022 (19.32):

using original_mapping_type = typename layout_stride::template mapping<OriginalExtents>;
using original_extents_type = typename original_mapping_type::extents_type; // ugly hack for compiling with MSVC 2022
using extents_type = transpose_extents_t<original_extents_type>;
stigrs commented 2 years ago

Hi again. One more change is required if compiling with /W4:

maybe_static_size.hpp(57,26): error: unused parameter 'val' 

Fixed if

__maybe_static_value(T val) noexcept { }

is changed to:

__maybe_static_value(T) noexcept { }
mhoemmen commented 2 years ago

@stigrs Thanks for reporting! I'll file this as an issue in the mdspan repository (https://github.com/kokkos/mdspan).

mhoemmen commented 2 years ago

Correction: This is actually a stdBLAS issue, not an mdspan issue.

KeithBallard commented 1 year ago

Sorry to resurrect this thread, but I am encountering the same issue with MSVC 2022. The current main branch (which has commit 5fafcdb) does not fix issue raised in the original post. However, I did find the same "ugly hack" employed by @stigrs works with VS 2022 17.4.3. Though I am not an expert in the nuances of typedef combined with typename, it does seem to be an error in MSVC. I will try to report it to Microsoft if I can replicate the issue with a simpler example.

However, I would like to know why the suggestion of @stigrs did not make it into the commit as it seems to be the fix to the actual issue? No worries if it was just an oversight, but I didn't want to try to fix it myself in the case I am missing something important.

mhoemmen commented 1 year ago

Reopening due to above comment.

mhoemmen commented 1 month ago

PR https://github.com/kokkos/stdBLAS/pull/277 includes the MSVC work-around and should fix this issue. Thanks for reporting! : - )