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

Remove the cmake option: onnxruntime_DEV_MODE #13573

Merged
merged 6 commits into from
Nov 7, 2022
Merged

Conversation

snnn
Copy link
Member

@snnn snnn commented Nov 5, 2022

Description

  1. Remove the cmake option onnxruntime_DEV_MODE and replace it with "--compile-no-warning-as-error"
  2. Suppress some GSL warnings because now we treat nvcc diag warnings as errors

Motivation and Context

#else
-#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
+#if defined(_MSC_VER) && !defined(__INTEL_COMPILER) && !defined(__NVCC__)
#define GSL_SUPPRESS(x) [[gsl::suppress(x)]]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvcc doesn't recognize this "gsl" attribute namespace.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I see how onnxruntime is probably the only GSL user to build it with nvcc, but on the other hand the patch is fairly small. Have you considered upstreaming it?

@snnn snnn marked this pull request as ready for review November 6, 2022 20:49
@snnn snnn merged commit efcbdac into main Nov 7, 2022
@snnn snnn deleted the snnn/remove_dev_mode branch November 7, 2022 17:06
henrywu2019 pushed a commit to henrywu2019/onnxruntime that referenced this pull request Dec 21, 2022
1. Remove the cmake option onnxruntime_DEV_MODE and replace it with
"--compile-no-warning-as-error"
2. Suppress some GSL warnings because now we treat nvcc diag warnings as
errors
URL https://github.com/microsoft/GSL/archive/refs/tags/v4.0.0.zip
URL_HASH SHA1=cf368104cd22a87b4dd0c80228919bb2df3e2a14
)
endif()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong, but the patch is safe to apply even when building with CUDA support turned off? What do you think about applying it unconditionally instead, so that both cuda-ful and cuda-free builds are built from "identical" source code?

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.

3 participants