Skip to content
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

Remove functions calling the deprecated _MM_SET_FLUSH_ZERO_MODE #912

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rainiwu
Copy link
Contributor

@rainiwu rainiwu commented Jan 26, 2024

From rust-lang/stdarch#1454, it seems that setting _MM_SET_FLUSH_ZERO_MODE results in "immediate Undefined Behavior", and as a result these functions were deprecated with 1.75.0. This PR removes the functions that include calls to _MM_SET_FLUSH_ZERO_MODE, fixing the warnings thrown when compiling on Rust versions 1.75.0 and newer.

@swfsql
Copy link
Contributor

swfsql commented Jan 29, 2024

The other error (error[E0277]: the trait bound cuda::device::Cuda: Conv1DKernel<f32> is not satisfied) seems to be related to this:

#[cfg(all(not(feature = "cudnn"), feature = "cuda"))]
mod cuda_kernel;

I'm new to cuda/cudnn, but it appears that the cuda implementation of Conv1DKernel is behind that cfg condition.
I believe that to solve this, the same condition (#[cfg(all(not(feature = "cudnn"), feature = "cuda"))]) should be present on the Conv1DKernel Device bound, relaxing it for the cudnn case:

// conv1d
+ super::super::conv1d::Conv1DKernel<E>

@coreylowman
Copy link
Owner

According to the new documentation of these functions, it seems like this functionality can be achieved with inline assembly? I'd much prefer that than entirely removing these functions.

@swfsql
Copy link
Contributor

swfsql commented Jan 31, 2024

I didn't read much material on this - but I'm afraid that every float operations would need to be implemented in asm (not sure)? if this is the case, I think this would require a lot of effort.

Could this be deferred to a later issue? Because if not, the CI would be blocking all prs until this is resolved.
From their discussion, they considered outright removing the functions (instead of deprecating it), so I think that leaving the code unchanged is not recommended.

@swfsql swfsql mentioned this pull request Feb 9, 2024
13 tasks
@swfsql swfsql mentioned this pull request Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants