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

build: streamline options for enabling LTC and MHLO #1221

Merged
merged 1 commit into from
Aug 13, 2022
Merged

build: streamline options for enabling LTC and MHLO #1221

merged 1 commit into from
Aug 13, 2022

Conversation

ashay
Copy link
Collaborator

@ashay ashay commented Aug 13, 2022

Prior to this patch, LTC was enabled by default, but it was disabled in
setup.py and in the Github CI rules. MHLO, on the other hand, was
enabled by default, but also (i.e. redundantly) enabled in setup.py and
in the Github CI rules.

This had the effect of LTC being left enabled unless the build is
triggered by setup.py or Github CI. It also had the effect of forcing
us to update the option in multiple places, whenever it needed to be
changed.

This patch sets the options' default value at the option definintion,
and it explicitly sets the value only when the default is overriden,
like for disabling MHLO in in-tree builds on macOS.

@ashay ashay changed the title build: streamling options for enabling LTC and MHLO build: streamline options for enabling LTC and MHLO Aug 13, 2022
Prior to this patch, LTC was enabled by default, but it was disabled in
setup.py and in the Github CI rules.  MHLO, on the other hand, was
enabled by default, but also (i.e. redundantly) enabled in setup.py and
in the Github CI rules.

This had the effect of LTC being left enabled unless the build is
triggered by setup.py or Github CI.  It also had the effect of forcing
us to update the option in multiple places, whenever it needed to be
changed.

This patch sets the options' default value at the option definintion,
and it explicitly sets the value only when the default is overriden,
like for disabling MHLO in in-tree builds on macOS.
@ashay ashay requested a review from powderluv August 13, 2022 05:31
@ashay
Copy link
Collaborator Author

ashay commented Aug 13, 2022

cc: @henrytwo @ZihengJiang

Copy link
Member

@sjain-stanford sjain-stanford left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@ashay ashay merged commit 606f4d2 into llvm:main Aug 13, 2022
@ashay ashay deleted the ashay/mhlo-ltc-options branch August 13, 2022 06:49
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
Signed-off-by: Ettore Tiotto <etiotto@ca.ibm.com>
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