-
Notifications
You must be signed in to change notification settings - Fork 396
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
base: master
Are you sure you want to change the base?
Conversation
Converting to draft while I investigate the x86 macOS failures |
ee1c24e
to
52c02b4
Compare
x86 macOS failure is #7181 |
@BradleyWood Can we get your review on this PR ? |
There was a problem hiding this 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.
@Spencer-Comin Wondering if this change is needed for OMR tests ? |
Jenkins build all |
@r30shah My port library changes in the tril tests rely on this change #7252 (comment) |
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, given that these changes are in x86 , I would like to request @BradleyWood / @0xdaryl to review and merge changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I am not sure if FMA is used by the compiler.
@BradleyWood I ran into a problem testing #7252 that was fixed by adding FMA here. I don't remember any more details; I'd have to do some experimentation if you want to know what the exact problem was. |
supports_feature_test
did not check that the returnedsupports*()
value matches the expected value, and instead directly return thesupports*()
value.OMR_FEATURE_X86_FMA
was not included insupports_feature_test