-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Run fewer math tests #20860
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
Run fewer math tests #20860
Conversation
test/test_other.py
Outdated
| self.do_runf('core/test_fcvt.cpp') | ||
|
|
||
| def test_llrint(self): | ||
| self.do_runf('core/test_llrint.c') |
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.
Move the source files and the .out files into other and use do_other_test here.
If you just use do_runf it doesn't look for an .out file.
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.
Thanks, done.
|
Is there ever a risk that these math function behave differently undef different opt levels. |
The ones implemented in libc should not, except if the binaryen optimizer or wasm2js affect them. For that reason I left some as But ones that are potentially compiler intrinsics like |
|
|
||
| @only_wasm2js('tests fmodf (which may use JS math)') | ||
| def test_math_fmodf(self): | ||
| self.do_run_in_out_file_test('math/fmodf.c') |
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.
Should these tests also be moved in other and run in wasm2js mode? Or do they benefit from being runing at different opt levels of wasm2js?
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.
I think these benefit from multiple opt levels, yeah. If we lower fmodf into JS % then the optimizer might do things with that. (However I don't think we actually do so, but it would be nice to someday.)
Move some to
otherwhen they are really just testing a libc method. Mark othersas wasm2js-only when they seem like they test bit operations or other things
that wasm2js needs coverage for (in multiple opt levels).
Remove
test_getgepas it was just testing trivial struct operations that havelots of other coverage (and it doesn't even test LLVM getelementptr directly).