Skip to content

Conversation

fbarchard
Copy link
Collaborator

@fbarchard fbarchard commented Aug 16, 2025

Instead of disabling neon dot on all unknown vendors, limit the disabling to known unisoc vendor.
Fixes a performance regression seen on Samsung and Pixel devices that have 'unknown' vendor.

On Samsung S23 Qualcomm medium core (A715)
SoC name: Unknown
Microarchitectures:
1x Cortex-X3
4x Cortex-A715
3x Cortex-A510

XNNPACK/bench/subgraph/benchmark --benchmark_filter=MobileNetV2
Was
QS8MobileNetV2/process_time/real_time 32116 us 31926 us 22 cpufreq=2.8032G
Now
QS8MobileNetV2/process_time/real_time 10531 us 10467 us 66 cpufreq=2.8032G

Fixes #322

Instead of disabling neon dot on all unknown vendors, limit the
disabling to known unisoc vendor.
Fixes a performance regression seen on Samsung and Pixel devices that
have 'unknown' vendor.
Copy link
Contributor

@kimishpatel kimishpatel left a comment

Choose a reason for hiding this comment

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

@GregoryComer this looks good to me? Are you ok with this?

@GregoryComer
Copy link
Member

@GregoryComer this looks good to me? Are you ok with this?

The chipset series field may not be populated in cases where the chipset is unknown. We should double check this to make sure it doesn't cause the UNISOC crashes to start again. I'm okay to merge this PR to unblock Samsung and Pixel devices, but we'll need to make a follow-up patch, if necessary, before pulling into fbsource.

I can take a look at the detection failure in more detail tomorrow to see if it would cause the series field to be empty.

@kimishpatel
Copy link
Contributor

@GregoryComer this looks good to me? Are you ok with this?

The chipset series field may not be populated in cases where the chipset is unknown. We should double check this to make sure it doesn't cause the UNISOC crashes to start again. I'm okay to merge this PR to unblock Samsung and Pixel devices, but we'll need to make a follow-up patch, if necessary, before pulling into fbsource.

I can take a look at the detection failure in more detail tomorrow to see if it would cause the series field to be empty.

Lets validate then before landing

@GregoryComer
Copy link
Member

@GregoryComer this looks good to me? Are you ok with this?

The chipset series field may not be populated in cases where the chipset is unknown. We should double check this to make sure it doesn't cause the UNISOC crashes to start again. I'm okay to merge this PR to unblock Samsung and Pixel devices, but we'll need to make a follow-up patch, if necessary, before pulling into fbsource.
I can take a look at the detection failure in more detail tomorrow to see if it would cause the series field to be empty.

Lets validate then before landing

It looks like chipset series will not be populated when the detection fails. Here's the line that's hit on the affected devices:

} else if (vendor != decoded_vendor) {
/* Parsing different system properties produces
* different chipset vendors. This situation is
* rare. */
cpuinfo_log_error(
"chipset detection failed: different chipset vendors reported in different system properties");
goto finish;
}

It bails out of detection and leaves the chipset series as unknown in this case. From a Meta-internal patch, I determined that the specific failure happens because it sees conflicting UNISOC and Spreadtrum vendors. To unblock this, we could probably update the detection logic to special case this. We could also try to use some other field to detect Unisoc in the feature detection logic, instead of the detected chipset, but this seems like it would get messy. I'm definitely open to better ideas.

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.

dot product disabled on aarch32
3 participants