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

Add user settings to BuildConfig #6

Merged
merged 3 commits into from
Aug 5, 2021

Conversation

schell
Copy link
Contributor

@schell schell commented Aug 5, 2021

Accompanies #5

  • changes TVMCMakeSettings to CMakeSettings
  • adds a UserSettings struct with most cmake settings from cmake.config
  • exposes UserSettings to being passed and parsed through command line args

@@ -316,6 +591,11 @@ impl Revision {
.host(&target.host)
.profile("Debug");

for (key, value) in build_config.as_cmake_define_key_values() {
println!("setting {}={}", key, value);
Copy link
Member

Choose a reason for hiding this comment

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

Is this debugging code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes - at first I wanted to make sure that the user could see their settings take flight - but then I noticed it's passed in the call to cmake anyway. I'll remove this in a follow up. Thanks for the fast review and merge 🙇 .

@jroesch
Copy link
Member

jroesch commented Aug 5, 2021

@schell can you address that one comment in a follow up PR? going to land

@jroesch jroesch merged commit e385f06 into octoml:tvm-config-opts Aug 5, 2021
@schell schell deleted the tvm-config-opts branch August 5, 2021 21:51
This was referenced Aug 5, 2021
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.

2 participants