Skip to content
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

Unconditionally re-enable /W4 on Windows and fix warnings which showed up #19588

Closed
wants to merge 2 commits into from

Conversation

mtavenrath
Copy link
Contributor

Description

I've set the compiler warning level for msvc back to /W4 for the CUDA backend to ensure better pre-commit testing and fixed the warnings in the core onnxruntime & CUDA EP which showed up after this change.

Motivation and Context

The warning level on gcc/clang is higher than the one on MSVC when enabling the CUDA backend resulting in CI potential failures when filing a PR developed on Windows. Increase the warning level to have more overlap in warnings between MSVC and gcc/clang.

fix issue #19565

@tianleiwu
Copy link
Contributor

/azp run Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-gpu-ci-pipeline, Linux MIGraphX CI Pipeline, Big Models

Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@snnn
Copy link
Member

snnn commented Feb 22, 2024

/azp run Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-gpu-ci-pipeline, Linux MIGraphX CI Pipeline, Big Models

Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@mtavenrath
Copy link
Contributor Author

When doing this change I had CUDA warnings disabled. Moving to /W4 without CUDA warnigns disabled opens a lot of new warnings, especially in CUTLASS. Closing for now until warning related issues are solved in CUTLASS:

@mtavenrath mtavenrath closed this Feb 23, 2024
@skottmckay
Copy link
Contributor

Could we call set_msvc_c_cpp_compiler_warning_level from onnxruntime_providers_cuda.cmake to set to 3 and the start and back to 4 at the end to minimize the code compiled with /W3? maybe we could just do that around the include(cutlass).

skottmckay added a commit that referenced this pull request Mar 6, 2024
### Description
<!-- Describe your changes. -->
Address warnings so all the ORT projects build with /W4 on Windows.

Mainly 
- unused parameters
- variables shadowing other ones

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
#19588 started on this.
zz002 pushed a commit to zz002/onnxruntime that referenced this pull request Mar 7, 2024
### Description
<!-- Describe your changes. -->
Address warnings so all the ORT projects build with /W4 on Windows.

Mainly 
- unused parameters
- variables shadowing other ones

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
microsoft#19588 started on this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants