-
Notifications
You must be signed in to change notification settings - Fork 201
Fix command line arguments for linker and compiler #895
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
Conversation
…g, and lineinfo. Fix ProgramOptions.__repr__() to return a string instead of a list.
/ok to test |
@mmason-nvidia, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
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.
LGTM, but should we do this for all of the boolean attributes for completeness?
(I'm new to the project, so I'm mostly suggesting this for the next reviewer to decide whether that makes sense).
@@ -244,8 +244,8 @@ def __post_init__(self): | |||
self._formatted_options.append("--device-debug") | |||
if self.lineinfo is not None and self.lineinfo: | |||
self._formatted_options.append("--generate-line-info") | |||
if self.device_code_optimize is not None: | |||
self._formatted_options.append(f"--dopt={'on' if self.device_code_optimize else 'off'}") | |||
if self.device_code_optimize is not None and self.device_code_optimize: |
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.
if self.device_code_optimize is not None and self.device_code_optimize: | |
if self.device_code_optimize is True: |
self.formatted_options.append("-g") | ||
if self.lineinfo is not None: | ||
if self.lineinfo is not None and self.lineinfo: |
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.
if self.lineinfo is not None and self.lineinfo: | |
if self.lineinfo is True: |
self.formatted_options.append("-lto") | ||
if self.ptx is not None: | ||
self.formatted_options.append("-ptx") | ||
if self.optimization_level is not None: | ||
self.formatted_options.append(f"-O{self.optimization_level}") | ||
if self.debug is not None: | ||
if self.debug is not None and self.debug: |
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.
if self.debug is not None and self.debug: | |
if self.debug is True: |
@@ -205,15 +205,15 @@ def _init_nvjitlink(self): | |||
self.formatted_options.append("-time") | |||
if self.verbose is not None: | |||
self.formatted_options.append("-verbose") | |||
if self.link_time_optimization is not None: | |||
if self.link_time_optimization is not None and self.link_time_optimization: |
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.
if self.link_time_optimization is not None and self.link_time_optimization: | |
if self.link_time_optimization is True: |
Probably a good idea! |
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.
As discussed offline, ignore my comments. I'll ask the copilot to do a global sweep later.
/ok to test 9f1282f |
|
Description
Fix command line arguments checking related to optimization, debugging, and lineinfo.
Fix ProgramOptions.repr() to return a string instead of a list.
Checklist