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

Quiet warnings about multiple optimization flags when they're repetitions of the same argument #4502

Merged
merged 1 commit into from
Nov 10, 2021
Merged

Quiet warnings about multiple optimization flags when they're repetitions of the same argument #4502

merged 1 commit into from
Nov 10, 2021

Conversation

PhilMiller
Copy link
Contributor

No description provided.

@dalg24
Copy link
Member

dalg24 commented Nov 4, 2021

Why should we treat duplicated optimization flags differently when they are duplicated?

@PhilMiller
Copy link
Contributor Author

Substantively, in the case where it's the same flag repeated (e.g. -O3 -O3 -O3), it's inconsequential that the duplicates are being stripped. The warning only indicates a problem to be fixed, where the user may not get what they were expecting, or has a broken/inconsistent configuration, if they ever differ.

Practically, EMPIRE's build system and its use of Trilinos ends up with exactly this sort of duplication on every single one of several thousand object files. It's pure noise in the output of every CUDA build. If there were some corner of the code (say a random TPL) in which the warning actually indicated a real problem, we wouldn't know, because we're not going to look past all the cases where it's not telling us anything interesting.

@PhilMiller
Copy link
Contributor Author

I'll admit that there's probably something subtly wrong in EMPIRE's build system leading to this (I know of many other things that are not so subtly wrong), but the effort to investigate and resolve it would be much lower priority than a few dozen other things that need work. If we still got this warning on a smaller subset of files after quieting down the noise, we'd have a much clearer reason to pursue the breakage.

@dalg24
Copy link
Member

dalg24 commented Nov 4, 2021

Substantively, in the case where it's the same flag repeated (e.g. -O3 -O3 -O3), it's inconsequential that the duplicates are being stripped. The warning only indicates a problem to be fixed, where the user may not get what they were expecting, or has a broken/inconsistent configuration, if they ever differ.

I disagree. I don't think that -O1 -O2 -O3 is any worse but this is my personal opinion and we can wait to see how others feel about this.

Would you consider an alternate solution that let you suppress that warning altogether (via an environment variable or some flag)?

@PhilMiller
Copy link
Contributor Author

I'd definitely consider an alternate solution that suppressed the warning in the exact duplication case.

@fnrizzi
Copy link
Contributor

fnrizzi commented Nov 10, 2021

Personally, I think in this case, duplicate flags is a problem caused by the application, so it should be up to app to fix it and provide the right flags, so I would vote for not stripping all duplicates but just giving a warning. And leaving it up to the app to disable that warning. Or, better, spend time fixing it :)

Something like: -DKokkos_DISABLE_WARNING_IF_REPEATED_OPTIMIZATION_FLAGS?

@fnrizzi
Copy link
Contributor

fnrizzi commented Nov 10, 2021

Arguing against myself, I would also say the it would not be such a burden for us to support stripping duplicates. So even that solution would not be too bad IMMO.

Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

Behaves similar to NVCC 11 which warns for different flags but not the same.
NVCC 9/10 error out with multiple optimization flags.

@dalg24 dalg24 merged commit b36ed92 into kokkos:develop Nov 10, 2021
@PhilMiller PhilMiller moved this from Done in or before release 3.5 to Done, pending release in Developer: Phil Miller Nov 10, 2021
@PhilMiller PhilMiller moved this from Done, pending release to Done in Release 3.6 in Developer: Phil Miller Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Developer: Phil Miller
Done in Release 3.6
Development

Successfully merging this pull request may close these issues.

None yet

4 participants