remove clang dep #711
Merged
remove clang dep #711
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
makslevental
force-pushed
the
remove_clang_dep
branch
from
2dc2ae3 to
4ebb71b
Compare
makslevental
requested review from
stephenneuendorffer,
fifield,
jgmelber,
erwei-xilinx and
jsetoain
keryell
reviewed
Nov 2, 2023
| @@ -2,17 +2,5 @@ | |||
| # Copyright (C) 2022, Advanced Micro Devices, Inc. All rights reserved. | |||
| # SPDX-License-Identifier: MIT | |||
|
|
|||
| # specify the compiler | |||
| set(CLANG_VER 12) | |||
There was a problem hiding this comment.
Great ! This one was a pain to me about looking for such a compiler in an antique boutique. So I always had to update this file.
jsetoain
approved these changes
Nov 2, 2023
jsetoain
approved these changes
Nov 2, 2023
makslevental
force-pushed
the
remove_clang_dep
branch
from
4ebb71b to
d2193e4
Compare
makslevental
commented
Nov 2, 2023
| if(CMAKE_CXX_COMPILER_ID MATCHES "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10.0") | ||
| check_cxx_compiler_flag("-Wno-unused-but-set-parameter" CXX_SUPPORTS_WNO_UNUSED_BUT_SET_PARAMETER) | ||
| append_if(CXX_SUPPORTS_WNO_UNUSED_BUT_SET_PARAMETER "-Wno-unused-but-set-parameter" CMAKE_CXX_FLAGS) | ||
| endif() |
There was a problem hiding this comment.
fifield
approved these changes
Nov 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the dependency on clang-12 for tests and examples. I say tests and examples because there was never a real dependency (I've never built with clang-12...) because as far as I understand the dependency was only on the artifacts passed to chess rather than any compilation aie artifacts themselves.
Note this PR also drops the
DCMAKE_COMPILE_WARNING_AS_ERRORbecause gcc will fail to compile MLIR with it on (lol).EDIT:
Just want to be clear: this works across all platforms with all compilers currently and I don't know why this was needed (before my time). The wheels I build for us are built using gcc on Linux, clang on Mac, and MSCV on Windows:
Regarding the
utils/scripts: there seems to be nothing to be gained from suggesting/implying/guaranteeing clang compatibility1 and not gcc. Yes, there are some clang specific#pragmas (and gcc too) butcc @nirvedhmeshram
Footnotes
And plenty to be lost (by signaling there's this added friction, of having clang, to using the tool). ↩