-
Notifications
You must be signed in to change notification settings - Fork 319
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
Disable neon dot on Unisoc UMS312 #265
Conversation
a72f52c
to
5ca8ae1
Compare
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, I assume you can trigger the debug log on the real device, right?
@@ -0,0 +1,31 @@ | |||
# begin build properties | |||
# autogenerated by buildinfo.sh |
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.
where is this 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.
It should be located under /system/build.props. However, I can't read it without root, so I pulled the template file and populated this by running adb shell getprop. I can remove this file, if desired.
@@ -147,6 +147,8 @@ void cpuinfo_arm_linux_decode_isa_from_proc_cpuinfo( | |||
"VDOT instructions disabled: cause occasional SIGILL on Spreadtrum SC9863A"); | |||
} else if (chipset->series == cpuinfo_arm_chipset_series_unisoc_t && chipset->model == 310) { | |||
cpuinfo_log_warning("VDOT instructions disabled: cause occasional SIGILL on Unisoc T310"); | |||
} else if (chipset->series == cpuinfo_arm_chipset_series_unisoc_ums && chipset->model == 312) { |
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.
we you able to repro this on armv8 kernel? And does this fix it? Ok with this change even if you couldn't TBH.
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.
Unfortunately. I don't have a device with UMS312 and armv8 kernel. From the crash logs, they were mostly (maybe all?) armv7 kernels, so it may be okay. If we see issues on armv8 kernels, I can take as a follow-up.
Yeah, I confirmed that it is working as intended on the Astro 55R by checking cpuinfo debug logging. With this change, it does disable dot instructions. |
This PR adds logic to recognize Unisoc UMS312 devices and disable neon dot instructions. This is due to SIGILL crashes on UMS312-based phones. We already disable dot instructions on the Unisoc T310, so this is not unprecedented. It is unclear to me if the UMS312 is actually a SOC or if is a board/platform including the T310. Regardless, it is identified separately, so I have added logic to handle it.
I've also added cpuinfo and ro.props for the Astro 55R, which is a UMS312-based device.
To validate this change, I added tests for matching T301 and UMS312 hardware strings and ran all tests on the Astro 55R. I noticed that there are a large number of existing failing tests, so I re-ran all tests on main and diffed the results to confirm that there are no regressions. I also built XNNPACK with this CPUINFO commit and validated that dot instructions were disabled via observing the relevant log message.