-
Notifications
You must be signed in to change notification settings - Fork 561
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
tool.record_filter_bycore_multi failured due to inaccurate pi estimation #6755
Comments
This passed on a re-run. It passes every time on the non-sve machine (don't have simple direct access to the sve machine). |
This issue happens only on sve it seems, but it is similar to a recent non-sve issue that was observed on signalNNN1 tests; where the result of some trig math floating-point ops changed and became non-deterministic after PR #6725. Since #6725 adjusted some sve and non-sve logic too, maybe both of these are indeed related. |
Fixes the slot used to save and restore FP regs at fcache enter and return events. PR #6725 adjusted the slots used during signal handling in core/unix/signal_linux_aarch64.c but did not adjust the same in fcache enter/return and attach events. Prior to that PR, the FP regs were simply stored in a contiguous manner in signal handling code and fcache enter/return routines (instead of in their respective dr_simd_t struct member), which was a bit confusing. The mismatch between slot usage in signal handling and fcache enter/return code caused failures in the signalNNN1 tests on some systems. Verified that those tests pass with this fix. Also fixes the same issue in save_priv_mcontext_helper which is used in the dr_app_start API. Unit tests for this scenario will be added as part of #6759. Issue: #5036, #6755, #5365 Fixes #6758
PR #6758 may help with this issue. I haven't used tmate to verify the failure count post that PR though. |
The tool.record_filter_bycore_multi test failed on a64 sve at https://github.com/DynamoRIO/dynamorio/actions/runs/8546804195/job/23417839931?pr=6754
The output mismatch looks like it's b/c it estimated pi at 3.94... instead of 3.14...
The text was updated successfully, but these errors were encountered: