Draft: Add filter rule( gcc 9 and older does not support C++ 20 ) #55
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
==========================================
+ Coverage 95.14% 95.32% +0.17%
==========================================
Files 10 10
Lines 701 727 +26
Branches 213 199 -14
==========================================
+ Hits 667 693 +26
- Misses 15 16 +1
+ Partials 19 18 -1 ☔ View full report in Codecov by Sentry. |
| @@ -596,3 +596,139 @@ def test_not_valid_cuda_versions_for_ubuntu_d4(self): | |||
| reason_msg.getvalue(), | |||
| f"host compiler clang-cuda {clang_cuda_version} is not available in Ubuntu 20.04", | |||
| ) | |||
|
|
|||
| def test_valid_gcc_cxx_combinations_d5(self): | |||
There was a problem hiding this comment.
The corner cases C++ 17 and C++ 20 are missing (see comment in Versions.py). Please test it also.
src/bashi/versions.py
Outdated
| # the latest supported cxx version must be added, even if the supported gcc version does not | ||
| # increase | ||
| GCC_CXX_SUPPORT: List[GccCxxSupport] = [ | ||
| GccCxxSupport("14", "23"), |
There was a problem hiding this comment.
I thought about this again. Actual the GCC 15 (not released yet) also adds missing C++23 and maybe the GCC 16 will too. So my idea is not good. Therefore we remove the entry for C++23 and replace it with C++17.
| GccCxxSupport("14", "23"), | |
| GccCxxSupport("8", "17"), |
The filter rule should still work. You need only to modify the tests. Sorry for the extra work.
src/bashi/versions.py
Outdated
| # increase | ||
| GCC_CXX_SUPPORT: List[GccCxxSupport] = [ | ||
| GccCxxSupport("14", "23"), | ||
| # GccCxxSupport("18", "26"), |
| # Rule: d5 | ||
| # remove all unsupported gcc cxx version combinations | ||
| """ | ||
| if DEVICE_COMPILER in row and row[DEVICE_COMPILER].name == GCC: |
There was a problem hiding this comment.
Don't forget to remove this.
There was a problem hiding this comment.
Okay, I'm just keeping it for the draft version
| break | ||
| """ | ||
|
|
||
| for compiler_type in (HOST_COMPILER, DEVICE_COMPILER): |
There was a problem hiding this comment.
Just as note, could be that you need to modify the rule, if you implement the C++ support filter rule for nvcc. Because with the nvcc filter rule, the C++ support of the host compiler depends on the device compiler. Small example:
If the host and device compiler is GCC 11, it supports C++20. If the host compiler is GCC 11 and the device Compiler is NVCC 11.8, only up to C++17 is allowed.
On the other side, the rule already removes invalid combinations which can never happen. If we check GCC 11 + C++ 23, it will never pass independent if the device compiler is GCC or NVCC. Therefore we keep it.
I only write down my thoughts in case you get a covertable.exceptions.InvalidCondition exception when you implement the nvcc C++ standard filter rule, that you have an point for under investigating.
|
|
||
| def test_remove_unsupported_cxx_versions_for_gcc(self): | ||
| test_param_value_pairs: List[ParameterValuePair] = parse_expected_val_pairs2( | ||
| [ |
There was a problem hiding this comment.
The corner cases ((HOST_COMPILER, GCC, 9), (CXX_STANDARD, 19)), and ((HOST_COMPILER, GCC, 9), (CXX_STANDARD, 20)), and ((HOST_COMPILER, GCC, 8), (CXX_STANDARD, 16)), and ((HOST_COMPILER, GCC, 8), (CXX_STANDARD, 17)), are missing.
src/bashi/results.py
Outdated
| removed_parameter_value_pairs: List[ParameterValuePair], | ||
| ): | ||
| for compiler_type in (HOST_COMPILER, DEVICE_COMPILER): | ||
| remove_parameter_value_pairs_ranges( |
There was a problem hiding this comment.
I think the usage of remove_parameter_value_pairs_ranges() is not correct yet. Because in the end, we should be able to iterate of the list GCC_CXX_SUPPORT and call the function remove_parameter_value_pairs_ranges() with each combination of the GCC version and CXX_STANDARD.
There was a problem hiding this comment.
Yes, it's a temporary solution, I wanted to write the right test_results first, and then change it to a more general case
No description provided.