Skip to content

Add ISA flag detection for s390x#4101

Merged
alexcrichton merged 1 commit intobytecodealliance:mainfrom
uweigand:isaflag-wasmtime
May 5, 2022
Merged

Add ISA flag detection for s390x#4101
alexcrichton merged 1 commit intobytecodealliance:mainfrom
uweigand:isaflag-wasmtime

Conversation

@uweigand
Copy link
Member

@uweigand uweigand commented May 5, 2022

Adds support for s390x to check_compatible_with_isa_flag,
which fixes running the test suite on z15 and later.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks!

_ => None,
}
}
#[cfg(all(target_arch = "s390x", not(target_os = "linux")))]
Copy link
Member

Choose a reason for hiding this comment

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

Instead of a maze of #[cfg] statements here, while you're at it could this be updated to use cfg_if!?

Either that or say above let mut enabled = None or something like that. Basically I don't think it's worth it to have lots of negated cfg's to handle this variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I tried the let mut enabled = None approach, which caused the compiler to complain about a value that was written, but not read, hence the current approach.

Copy link
Member

Choose a reason for hiding this comment

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

With a compiler warning I think it's fine to have a local #[allow] for something like this, that's easier to maintain than a complicated #[cfg] clause

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, updated patch along those lines.

Adds support for s390x to check_compatible_with_isa_flag,
which fixes running the test suite on z15 and later.
@uweigand uweigand force-pushed the isaflag-wasmtime branch from 7e7ab66 to 808c608 Compare May 5, 2022 15:52
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label May 5, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

Subscribe to Label Action

cc @peterhuene

Details This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@alexcrichton alexcrichton merged commit e1f7b50 into bytecodealliance:main May 5, 2022
@uweigand uweigand deleted the isaflag-wasmtime branch May 5, 2022 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasmtime:api Related to the API of the `wasmtime` crate itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants