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

Clean up x86 supports_feature_test #7258

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Spencer-Comin
Copy link
Contributor

  • Some case branches in supports_feature_test did not check that the returned supports*() value matches the expected value, and instead directly return the supports*() value.
  • OMR_FEATURE_X86_FMA was not included in supports_feature_test

@Spencer-Comin
Copy link
Contributor Author

Spencer-Comin commented Mar 15, 2024

Converting to draft while I investigate the x86 macOS failures

Some cases were missing the check that they returned the expected value.

Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
@Spencer-Comin
Copy link
Contributor Author

x86 macOS failure is #7181

@Spencer-Comin Spencer-Comin marked this pull request as ready for review April 5, 2024 13:37
@r30shah
Copy link
Contributor

r30shah commented Apr 8, 2024

@BradleyWood Can we get your review on this PR ?

Copy link
Member

@BradleyWood BradleyWood left a comment

Choose a reason for hiding this comment

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

Looks OK, but I'll add that the old api will eventually be removed in favor of the port library.

@r30shah
Copy link
Contributor

r30shah commented Apr 9, 2024

@Spencer-Comin Wondering if this change is needed for OMR tests ?

@r30shah
Copy link
Contributor

r30shah commented Apr 9, 2024

Jenkins build all

@Spencer-Comin
Copy link
Contributor Author

@r30shah My port library changes in the tril tests rely on this change #7252 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants