Skip to content

Conversation

@marcinz
Copy link
Collaborator

@marcinz marcinz commented Jan 19, 2023

Change the default architecture to haswell. Currently, we build our conda packages with haswell to avoid compatibility issues. Changing the default architecture to haswell will make default builds comparable to the conda packages and it will allow developers to share sccache with CI builds.

@manopapad
Copy link
Contributor

This should be the default only when building on an x86 machine.

Please add a comment for why we're doing this in the script itself.

This should also be changed on the cunumeric side.

@marcinz marcinz requested a review from manopapad January 19, 2023 23:05
@marcinz
Copy link
Collaborator Author

marcinz commented Jan 19, 2023

@manopapad Do we have a check whether we are building on an x86 machine? If we do not, maybe this is not a good idea.

@manopapad
Copy link
Contributor

I think it could just be

default=("haswell" if platform.machine() == "x86_64" else "native"),

but make @bryevdv @m3vaz have a better suggestion?

@bryevdv
Copy link
Contributor

bryevdv commented Jan 20, 2023

Seems reasonable to me

@trxcllnt trxcllnt added the category:improvement PR introduces an improvement and will be classified as such in release notes label Jan 20, 2023
install.py Outdated
dest="march",
required=False,
default="native",
default="haswell",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
default="haswell",
default=("haswell" if platform.machine() == "x86_64" else "native"),

@marcinz marcinz merged commit e537b10 into nv-legate:branch-22.12 Jan 23, 2023
manopapad pushed a commit that referenced this pull request Mar 5, 2025
updating legion hash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:improvement PR introduces an improvement and will be classified as such in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants