Skip to content

add ElementaryFunctions conformance to SIMD types #312

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

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

Conversation

JaapWijnen
Copy link

Adresses #103
There's a few parts that probably require discussion.

ElementaryFunctions requires conformance to AdditiveArithmetic
The concrete SIMD2, 4, etc types from the standard library have a conformance to AdditiveArithmetic defined in the Differentiation module so this conformance is only visible when _Differentiation is imported.
It's defined as follows: extension SIMD${n}: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}

In some of our code that would be used with swift toolchains both with and without access to the _Differentiation module we'd get around this by using the following:

#if canImport(_Differentiation)
import _Differentiation
#endif

#if !canImport(_Differentiation)
// add `AdditiveArithmetic` conformance since this is only present in the _Differentiation module which is not present everywhere
extension SIMD2: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}
extension SIMD4: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}
extension SIMD8: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}
extension SIMD16: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}
extension SIMD32: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}
extension SIMD64: @retroactive AdditiveArithmetic where Scalar: FloatingPoint {}
#endif

My main worry here is that the conformance to AdditiveArithmetic in the standard library will conflict with the one provided here. Either the full one or the one that's limited to types conforming to FloatingPoint. But as far as I understand this should be ok due to the @retroactive attribute.

Furthermore, technically we could be more generic (here and in the stdlib _Differentiation module) by requiring Scalar to conform to just AdditiveArithmetic but that would require more boilerplate, now we get the implementation for free from the standard library. The other solution is more complete however and would look something like:

extension SIMD2: @retroactive AdditiveArithmetic where Scalar: AdditiveArithmetic {
    @_transparent
    public static var zero: SIMD2<Scalar> {
        Self()
    }
    
    @_transparent
    public static func + (lhs: SIMD2<Scalar>, rhs: SIMD2<Scalar>) -> SIMD2<Scalar> {
        var v = Self()
        for i in v.indices {
            v[i] = lhs[i] + rhs[i]
        }
        return v
    }
    
    @_transparent
    public static func - (lhs: SIMD2<Scalar>, rhs: SIMD2<Scalar>) -> SIMD2<Scalar> {
        var v = Self()
        for i in v.indices {
            v[i] = lhs[i] - rhs[i]
        }
        return v
    }
}

Ideally the AdditiveArithmetic conformance would live in the stdlib itself. That way none of the Differentiation code would break and there's no longer the need for this duplication. But I'm not sure if just a "completeness" argument warrants that change.

Aside from the above I also added implementations for most of the RealFunction except for static func signGamma(_ x: Self) -> FloatingPointSign since it can't implemented for simd types. So unfortunately we can't conform the simdtypes to the protocol.

Let me know your thoughts/questions!

@JaapWijnen
Copy link
Author

@stephentyrone in case you hadn't seen this yet. Just wanted to see if you had any thoughts on this. No rush though!

@stephentyrone
Copy link
Member

I'm currently pondering this, and will likely have some questions about it after I finish tagging 1.1

@JaapWijnen
Copy link
Author

Of course! Let me know your thoughts once you have some time.

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.

2 participants