Check extended CPU features only when necessary as querying flags fail on some AMD CPUs.#426
Conversation
…l on some AMD CPUs . Some AMD CPUs fail on querying CPUID extended flags. See https://bugzilla.mozilla.org/show_bug.cgi?id=1615786. Performing this check only when actually required, allows wasm modules that don't rely on extended features to be run.
| let info = cpuid.get_extended_feature_info().ok_or_else(|| { | ||
| Error::Unsupported("Unable to obtain host CPU extended feature info!".to_string()) | ||
| })?; | ||
| if module_features.bmi1 || module_features.bmi2 { |
There was a problem hiding this comment.
| if module_features.bmi1 || module_features.bmi2 { | |
| // Some AMD CPUs fail on querying CPUID extended flags. See https://bugzilla.mozilla.org/show_bug.cgi?id=1615786. | |
| if module_features.bmi1 || module_features.bmi2 { |
| let info = cpuid.get_extended_function_info().ok_or_else(|| { | ||
| Error::Unsupported("Unable to obtain host CPU extended function info!".to_string()) | ||
| })?; | ||
| if module_features.lzcnt { |
There was a problem hiding this comment.
| if module_features.lzcnt { | |
| // Same situation - see above | |
| if module_features.lzcnt { |
|
I messed up the review thing - looks good, if you can approve adding those suggested comments we'll get it merged. |
There was a problem hiding this comment.
Looks good, this isn't actually an AMD CPU-specific issue - I think we'd hit this on processors older than ~2012 generally. For reference, the model number in the associated crash is family 16 model 10 stepping 0 which corresponds to a Phenom II X6, which shipped in 2010.
get_extended_feature_info checks CPUID leaf eax=7; I can't tell when exactly it was introduced, 2012 is an estimate. I'd originally thought it checked eax=1 (present on all processors since the 486). That's probably what is None on the reporter's machine, and causes an error. By any chance, do you get the error out of Lucet anywhere? I'd like to confirm that. Even so, I wouldn't think that's a blocking issue here since the change here has us match how feature detection is done in cranelift-native.
|
pt2: on account of not being AMD-specific, I'm going to squash merge and rewrite the commit message to reference the bug report and having made a stronger assumption than we should about CPUID leaf availability. |
3 participants
Some AMD CPUs fail on querying CPUID extended flags. See https://bugzilla.mozilla.org/show_bug.cgi?id=1615786. Performing this check only when actually required, allows wasm modules that don't rely on extended features to be run.