-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft: Add filter rule( gcc 9 and older does not support C++ 20 ) #55
base: main
Are you sure you want to change the base?
Changes from 1 commit
bfd6ab9
eb341ef
0cddfef
89327ba
681e7e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
from bashi.types import ParameterValueTuple | ||
from bashi.globals import * # pylint: disable=wildcard-import,unused-wildcard-import | ||
from bashi.utils import reason | ||
from bashi.versions import get_oldest_supporting_clang_version_for_cuda | ||
from bashi.versions import get_oldest_supporting_clang_version_for_cuda, GCC_CXX_SUPPORT | ||
|
||
|
||
def __ubuntu_version_to_string(version: pkv.Version) -> str: | ||
|
@@ -156,4 +156,58 @@ def software_dependency_filter( | |
f"{__ubuntu_version_to_string(row[UBUNTU].version)}", | ||
) | ||
return False | ||
|
||
# Rule: d5 | ||
# remove all unsupported gcc cxx version combinations | ||
""" | ||
if DEVICE_COMPILER in row and row[DEVICE_COMPILER].name == GCC: | ||
if CXX_STANDARD in row: | ||
if row[DEVICE_COMPILER].version <= GCC_CXX_SUPPORT[0].gcc_version: | ||
# check the maximum supported gcc version for the given cxx version | ||
for gcc_cxx_comb in GCC_CXX_SUPPORT: | ||
if row[DEVICE_COMPILER].version >= gcc_cxx_comb.gcc_version: | ||
if row[CXX_STANDARD].version >= gcc_cxx_comb.cxx_version: | ||
reason( | ||
output, | ||
"device compiler" | ||
f" gcc {row[DEVICE_COMPILER].version} " | ||
f"does not support cxx {row[CXX_STANDARD].version}" | ||
) | ||
return False | ||
break | ||
|
||
if HOST_COMPILER in row and row[HOST_COMPILER].name == GCC: | ||
if CXX_STANDARD in row: | ||
if row[HOST_COMPILER].version <= GCC_CXX_SUPPORT[0].gcc_version: | ||
# check the maximum supported gcc version for the given cxx version | ||
for gcc_cxx_comb in GCC_CXX_SUPPORT: | ||
if row[HOST_COMPILER].version >= gcc_cxx_comb.gcc_version: | ||
if row[CXX_STANDARD].version >= gcc_cxx_comb.cxx_version: | ||
reason( | ||
output, | ||
"host compiler" | ||
f" gcc {row[HOST_COMPILER].version} " | ||
f"does not support cxx {row[CXX_STANDARD].version}" | ||
) | ||
return False | ||
break | ||
""" | ||
|
||
for compiler_type in (HOST_COMPILER, DEVICE_COMPILER): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as note, could be that you need to modify the rule, if you implement the C++ support filter rule for I only write down my thoughts in case you get a |
||
if compiler_type in row and row[compiler_type].name == GCC: | ||
if CXX_STANDARD in row: | ||
if row[compiler_type].version <= GCC_CXX_SUPPORT[0].gcc_version: | ||
# check the maximum supported gcc version for the given cxx version | ||
for gcc_cxx_comb in GCC_CXX_SUPPORT: | ||
if row[compiler_type].version >= gcc_cxx_comb.gcc_version: | ||
if row[CXX_STANDARD].version >= gcc_cxx_comb.cxx_version: | ||
reason( | ||
output, | ||
f"{__pretty_name_compiler(compiler_type)}" | ||
f" gcc {row[compiler_type].version} " | ||
f"does not support cxx {row[CXX_STANDARD].version}", | ||
) | ||
return False | ||
break | ||
|
||
return True |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,6 +83,7 @@ def get_expected_bashi_parameter_value_pairs( | |
param_val_pair_list, removed_param_val_pair_list | ||
) | ||
_remove_unsupported_cuda_versions_for_ubuntu(param_val_pair_list, removed_param_val_pair_list) | ||
_remove_unsupported_cxx_versions_for_gcc(param_val_pair_list, removed_param_val_pair_list) | ||
return (param_val_pair_list, removed_param_val_pair_list) | ||
|
||
|
||
|
@@ -908,3 +909,35 @@ def _remove_unsupported_cuda_versions_for_ubuntu( | |
value_max_version2=12, | ||
value_max_version2_inclusive=False, | ||
) | ||
|
||
|
||
def _remove_unsupported_cxx_versions_for_gcc( | ||
parameter_value_pairs: List[ParameterValuePair], | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the usage of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's a temporary solution, I wanted to write the right test_results first, and then change it to a more general case |
||
parameter_value_pairs, | ||
removed_parameter_value_pairs, | ||
parameter1=compiler_type, | ||
value_name1=GCC, | ||
value_min_version1=10, | ||
value_max_version1=14, | ||
value_max_version1_inclusive=False, | ||
parameter2=CXX_STANDARD, | ||
value_name2=CXX_STANDARD, | ||
value_min_version2=20, | ||
value_min_version2_inclusive=True, | ||
) | ||
for compiler_type in (HOST_COMPILER, DEVICE_COMPILER): | ||
remove_parameter_value_pairs_ranges( | ||
parameter_value_pairs, | ||
removed_parameter_value_pairs, | ||
parameter1=compiler_type, | ||
value_name1=GCC, | ||
value_min_version1=14, | ||
parameter2=CXX_STANDARD, | ||
value_name2=CXX_STANDARD, | ||
value_min_version2=24, | ||
value_min_version2_inclusive=True, | ||
) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -64,6 +64,23 @@ def __str__(self) -> str: | |||||
return f"Clang-CUDA {str(self.clang_cuda)} + CUDA SDK {self.cuda}" | ||||||
|
||||||
|
||||||
# pylint: disable=too-few-public-methods | ||||||
class GccCxxSupport(VersionSupportBase): | ||||||
"""Contains a nvcc version and host compiler version. Does automatically parse the input strings | ||||||
to package.version.Version. | ||||||
|
||||||
Provides comparision operators for sorting. | ||||||
""" | ||||||
|
||||||
def __init__(self, gcc_version: str, cxx_version: str): | ||||||
VersionSupportBase.__init__(self, gcc_version, cxx_version) | ||||||
self.gcc_version: packaging.version.Version = self.version1 | ||||||
self.cxx_version: packaging.version.Version = self.version2 | ||||||
|
||||||
def __str__(self) -> str: | ||||||
return f"GCC {str(self.gcc_version)} + CXX {self.cxx_version}" | ||||||
|
||||||
|
||||||
VERSIONS: Dict[str, List[Union[str, int, float]]] = { | ||||||
GCC: [6, 7, 8, 9, 10, 11, 12, 13], | ||||||
CLANG: [6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17], | ||||||
|
@@ -123,6 +140,16 @@ def __str__(self) -> str: | |||||
] | ||||||
NVCC_GCC_MAX_VERSION.sort(reverse=True) | ||||||
|
||||||
|
||||||
# define the maximum supported gcc version for a specific cxx version | ||||||
# 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
The filter rule should still work. You need only to modify the tests. Sorry for the extra work. |
||||||
# GccCxxSupport("18", "26"), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the comment. |
||||||
GccCxxSupport("10", "20"), | ||||||
] | ||||||
GCC_CXX_SUPPORT.sort(reverse=True) | ||||||
# define the maximum supported clang version for a specific nvcc version | ||||||
# the latest supported nvcc version must be added, even if the supported clang version does not | ||||||
# increase | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The corner cases C++ 17 and C++ 20 are missing (see comment in |
||
self.assertTrue( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
HOST_COMPILER: ppv((GCC, 10)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, 18)), | ||
} | ||
) | ||
) | ||
) | ||
self.assertTrue( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
DEVICE_COMPILER: ppv((GCC, 10)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, 19)), | ||
} | ||
) | ||
) | ||
) | ||
self.assertTrue( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
HOST_COMPILER: ppv((GCC, 14)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, 22)), | ||
} | ||
) | ||
) | ||
) | ||
self.assertTrue( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
DEVICE_COMPILER: ppv((GCC, 14)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, 22)), | ||
} | ||
) | ||
) | ||
) | ||
|
||
self.assertTrue( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
HOST_COMPILER: ppv((GCC, 18)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, 25)), | ||
} | ||
) | ||
) | ||
) | ||
self.assertTrue( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
DEVICE_COMPILER: ppv((GCC, 18)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, 25)), | ||
} | ||
) | ||
) | ||
) | ||
|
||
def test_invalid_gcc_cxx_combinations_d5(self): | ||
for cxx_version in [20, 21, 28]: | ||
reason_msg = io.StringIO() | ||
self.assertFalse( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
HOST_COMPILER: ppv((GCC, 10)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, cxx_version)), | ||
} | ||
), | ||
reason_msg, | ||
), | ||
) | ||
self.assertEqual( | ||
reason_msg.getvalue(), | ||
f"host compiler gcc 10 does not support cxx {cxx_version}", | ||
) | ||
|
||
for cxx_version in [20, 21, 28]: | ||
reason_msg = io.StringIO() | ||
self.assertFalse( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
DEVICE_COMPILER: ppv((GCC, 10)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, cxx_version)), | ||
} | ||
), | ||
reason_msg, | ||
), | ||
) | ||
self.assertEqual( | ||
reason_msg.getvalue(), | ||
f"device compiler gcc 10 does not support cxx {cxx_version}", | ||
) | ||
|
||
for cxx_version in [24, 25, 30]: | ||
reason_msg = io.StringIO() | ||
self.assertFalse( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
DEVICE_COMPILER: ppv((GCC, 14)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, cxx_version)), | ||
} | ||
), | ||
reason_msg, | ||
), | ||
) | ||
self.assertEqual( | ||
reason_msg.getvalue(), | ||
f"device compiler gcc 14 does not support cxx {cxx_version}", | ||
) | ||
|
||
for cxx_version in [24, 25, 30]: | ||
reason_msg = io.StringIO() | ||
self.assertFalse( | ||
software_dependency_filter_typechecked( | ||
OD( | ||
{ | ||
HOST_COMPILER: ppv((GCC, 14)), | ||
CXX_STANDARD: ppv((CXX_STANDARD, cxx_version)), | ||
} | ||
), | ||
reason_msg, | ||
), | ||
) | ||
self.assertEqual( | ||
reason_msg.getvalue(), | ||
f"host compiler gcc 14 does not support cxx {cxx_version}", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'm just keeping it for the draft version