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

[SR-9848] SIMD Self recursion breaks SIL: requirement Self parameter must conform to exactly one protocol #52259

Closed
atrick opened this issue Feb 3, 2019 · 7 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@atrick
Copy link
Member

atrick commented Feb 3, 2019

Previous ID SR-9848
Radar rdar://47767506
Original Reporter @atrick
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee @DougGregor
Priority Medium

md5: 311ea4ef0fac73fcbc4fb3e05b2d86e9

Issue Description:

This is a bug in the type system that users can easily hit with Swift's SIMDVector library. I can see where the compiler is incorrect, but I'm not sure what the right fix should be...

// stdlib's SIMDVector does something like this...
public protocol DummyProtocol {
 // Interestingly this protocol might also define:
 // func abs() -> Self
}
public protocol SIMDStorageStub {
 associatedtype Scalar : DummyProtocol
}
// Another protocol in SIMDVector.swift introduces a recursive constraint.
// That seems ok in itself, but the extra conformance on DummyProtocol breaks
// witness method invocation at the SIL level.
public protocol SIMDScalarStub {
 associatedtype SIMD2Storage : SIMDStorageStub where SIMD2Storage.Scalar == Self
}
// Now we have an app that wants to conform to this protocol...
// Everything up to here could be replaced by:
// import simd
// /s/SIMDScalarStub/SIMDScalar
public protocol FPScalar: SIMDScalarStub {
 func abs() -> Self
}
public func callAbs<T: FPScalar>(s: T) -> T {
 return s.abs() // Are we supposed to call a method on FPScalar or DummyProtocol and does it matter?
}

The SILVerifier checks that witness methods conformances correspond to a single protocol decl:

auto conformsTo = genericSig->getConformsTo(selfGenericParam);
 require(conformsTo.size() == 1,
 "requirement Self parameter must conform to exactly one protocol"); 
SIL verification failed: requirement Self parameter must conform to exactly one protocol: conformsTo.size() == 1
Verifying instruction:
-> %3 = witness_method $T, #FPScalar.abs!1 : <Self where Self : FPScalar> (Self) -> () -> @dynamic_self Self : $@convention(witness_method: FPScalar) <τ_0_0 where τ_0_0 : FPScalar> (@in_guaranteed τ_0_0) -> @out τ_0_0 // user: %4
 %4 = apply %3<T>(%0, %1) : $@convention(witness_method: FPScalar) <τ_0_0 where τ_0_0 : FPScalar> (@in_guaranteed τ_0_0) -> @out τ_0_0
In function:
// callAbs<A>(s:)
sil [ossa] @$s11testconform7callAbs1sxx_tAA8FPScalarRzlF : $@convention(thin) <T where T : FPScalar> (@in_guaranteed T) -> @out T {

The GenericSignatureBuilder considers this the representative potential archetype:

τ_0_0: FPScalar *[τ_0_0: Explicit]
 & SIMDScalarStub [τ_0_0: Explicit -> Protocol requirement (via Self in FPScalar)]
 & DummyProtocol [τ_0_0: Explicit -> Protocol requirement (via Self in FPScalar)
 -> Protocol requirement (via Self.SIMD2Storage in SIMDScalarStub)
 -> Protocol requirement (via Self.Scalar in SIMDStorageStub)]
 [equivalence class τ_0_0[.SIMDScalarStub].SIMD2Storage[.SIMDStorageStub].Scalar]
 SIMD2Storage: SIMDStorageStub [τ_0_0: Explicit -> Protocol requirement (via Self in FPScalar) -> Protocol requirement (via Self.SIMD2Storage in SIMDScalarStub)]
 Scalar [represented by τ_0_0] [equivalence class τ_0_0]

The generic parameter conforms to three protocols:

  • FPScalar

  • SIMDScalarStub

  • DummyProtocol

ProtocolType::canonicalizeProtocols removes SIMDScalarStub because it is in the inheritance hierarchy. DummyProtocol is not in the inheritance hierarchy.

I assume the SILVerifier check exists for a reason but have no way of knowing where this invariant is assumed throughout the compiler...

This could be fixed by adding a bunch of complexity to canonicalizeProtocols, but is there a simpler solution and does this case have broader implications for the type system?

@atrick
Copy link
Member Author

atrick commented Feb 3, 2019

@slavapestov added this SILVerifier requirement. Insights?

@atrick
Copy link
Member Author

atrick commented Feb 3, 2019

FWIW, the same assert is in `SILFunctionType::getDefaultWitnessMethodProtocol`

@slavapestov
Copy link
Member

It looks like the generic signature is not minimized correctly. Since 'Self == SIMD2Storage.Scalar' can be derived from the protocol's requirement signature, so can 'Self : DummyProtocol', and therefore the generic signature of the protocol requirement should not contain any additional constraints on 'Self'.

@atrick
Copy link
Member Author

atrick commented Feb 4, 2019

If ProtocolType::canonicalizeProtocols is what you mean by "minimizing" the signature, then yes, it's failing to strip 'DummyProtocol' because it's just doing a dumb search up the inheritance chain.

@slavapestov
Copy link
Member

Oh I see what you mean. Yeah, it might not be a GSB bug. In any case I'll take a look.

@DougGregor
Copy link
Member

Slightly more reduced:

public protocol DummyProtocol { }public protocol SIMDStorageStub {
 associatedtype Scalar : DummyProtocol
}

// Another protocol in SIMDVector.swift introduces a recursive constraint.
// That seems ok in itself, but the extra conformance on DummyProtocol breaks
// witness method invocation at the SIL level.
public protocol SIMDScalarStub {
 associatedtype SIMD2Storage : SIMDStorageStub
   where SIMD2Storage.Scalar == Self
 
 func abs() -> Self
}

public func callAbs<T: SIMDScalarStub>(s: T) -> T {
 return s.abs()
} 

@DougGregor
Copy link
Member

It's not a GSB bug, but an incorrect assumption at the SIL level. Fix is at #22347

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

3 participants