SILVerifier: Should SuperMethodInst's result type be the same type as the referenced member?

[Joe, Roman, sorry for resend, I got the e-mail group wrong]

In SuperMethodInst's verifier, we have:

require(CMI->getType() == TC.getConstantType(CMI->getMember()),
       "result type of super_method must match type of method");

I think this assumption was valid when we only allowed super_method on foreign classes, without needing to worry about reabstractions. Now that we're allowing super_method with operands of native class type which can be generic, does this check make sense anymore?

Consider the following:

class Parent<A> {
let x: A
required init(x: A) { self.x = x }
}

class Child : Parent<String> {
required init(x: String) {
   super.init(x: x)
}
}

class Grandchild : Child {}

Here, the vtable thunks for their initializers have respective types:

$@convention(method) <T> (@in T, @owned Base<T>) -> @owned Base<T>
$@convention(method) (@in String, @owned Child) -> @owned Child
$@convention(method) (@in String, @owned Grandchild) -> @owned Grandchild

However, the real backing implementations have these respective types:

$@convention(method) <T> (@in T, @owned Base<T>) -> @owned Base<T>
$@convention(method) (@owned String, @owned Child) -> @owned Child
$@convention(method) (@owned String, @owned Grandchild) -> @owned Grandchild

So, Child and Grandchild have abstraction differences because their initializers aren't generic. When I make a super_method instruction, the constant appears to always point to the backing implementation, not the thunk, so I needed to get the overridden vtable entry from the constant and I think that's reasonable. That gives me:

super_method %10 : $Child, #Base.init!initializer.1 : <T> Base<T>.Type -> (x: T) -> Base<T> , $@convention(method) <τ_0_0> (@in τ_0_0, @owned Base<τ_0_0>) -> @owned Base<τ_0_0>

and

super_method %6 : $Grandchild, #Child.init!initializer.1 : Child.Type -> (x: String) -> Child , $@convention(method) (@in String, @owned Child) -> @owned Child

which look good to me.

With my changes today to fix generic substitutions of partial super methods and getting the right type from the vtable, if I disable that verifier check, devirtualization works correctly with super_method instructions.

Is this a problem with SILDeclRef or is this check simply no longer valid in the verifier? If so, I wonder what the suitable replacement check should be. Maybe something like:

if the constant is foreign:
do the original check
else:
require super_method's result type == vtable entry's function type (as opposed to the backing implementation)

David

I’d look at the verifier for ClassMethodInst. It turns out that TypeConverter has a getConstantOverrideType that lowers according to the overridden abstraction pattern.

John.

···

On Dec 9, 2015, at 6:09 PM, David Farler via swift-dev <swift-dev@swift.org> wrote:
[Joe, Roman, sorry for resend, I got the e-mail group wrong]

In SuperMethodInst's verifier, we have:

require(CMI->getType() == TC.getConstantType(CMI->getMember()),
       "result type of super_method must match type of method");

I think this assumption was valid when we only allowed super_method on foreign classes, without needing to worry about reabstractions. Now that we're allowing super_method with operands of native class type which can be generic, does this check make sense anymore?

Consider the following:

class Parent<A> {
let x: A
required init(x: A) { self.x = x }
}

class Child : Parent<String> {
required init(x: String) {
   super.init(x: x)
}
}

class Grandchild : Child {}

Here, the vtable thunks for their initializers have respective types:

$@convention(method) <T> (@in T, @owned Base<T>) -> @owned Base<T>
$@convention(method) (@in String, @owned Child) -> @owned Child
$@convention(method) (@in String, @owned Grandchild) -> @owned Grandchild

However, the real backing implementations have these respective types:

$@convention(method) <T> (@in T, @owned Base<T>) -> @owned Base<T>
$@convention(method) (@owned String, @owned Child) -> @owned Child
$@convention(method) (@owned String, @owned Grandchild) -> @owned Grandchild

So, Child and Grandchild have abstraction differences because their initializers aren't generic. When I make a super_method instruction, the constant appears to always point to the backing implementation, not the thunk, so I needed to get the overridden vtable entry from the constant and I think that's reasonable. That gives me:

super_method %10 : $Child, #Base.init!initializer.1 : <T> Base<T>.Type -> (x: T) -> Base<T> , $@convention(method) <τ_0_0> (@in τ_0_0, @owned Base<τ_0_0>) -> @owned Base<τ_0_0>

and

super_method %6 : $Grandchild, #Child.init!initializer.1 : Child.Type -> (x: String) -> Child , $@convention(method) (@in String, @owned Child) -> @owned Child

which look good to me.

With my changes today to fix generic substitutions of partial super methods and getting the right type from the vtable, if I disable that verifier check, devirtualization works correctly with super_method instructions.

Is this a problem with SILDeclRef or is this check simply no longer valid in the verifier? If so, I wonder what the suitable replacement check should be. Maybe something like:

Yep, that's what I moved over to using yesterday to fix the abstraction mismatch in the devirtualizer and that's when I started getting the verifier error. I'll take a look at the class method's verifier and see what it says.

David

···

On Dec 10, 2015, at 08:16, John McCall <rjmccall@apple.com> wrote:

On Dec 9, 2015, at 6:09 PM, David Farler via swift-dev <swift-dev@swift.org> wrote:
[Joe, Roman, sorry for resend, I got the e-mail group wrong]

In SuperMethodInst's verifier, we have:

require(CMI->getType() == TC.getConstantType(CMI->getMember()),
       "result type of super_method must match type of method");

I think this assumption was valid when we only allowed super_method on foreign classes, without needing to worry about reabstractions. Now that we're allowing super_method with operands of native class type which can be generic, does this check make sense anymore?

Consider the following:

class Parent<A> {
let x: A
required init(x: A) { self.x = x }
}

class Child : Parent<String> {
required init(x: String) {
   super.init(x: x)
}
}

class Grandchild : Child {}

Here, the vtable thunks for their initializers have respective types:

$@convention(method) <T> (@in T, @owned Base<T>) -> @owned Base<T>
$@convention(method) (@in String, @owned Child) -> @owned Child
$@convention(method) (@in String, @owned Grandchild) -> @owned Grandchild

However, the real backing implementations have these respective types:

$@convention(method) <T> (@in T, @owned Base<T>) -> @owned Base<T>
$@convention(method) (@owned String, @owned Child) -> @owned Child
$@convention(method) (@owned String, @owned Grandchild) -> @owned Grandchild

So, Child and Grandchild have abstraction differences because their initializers aren't generic. When I make a super_method instruction, the constant appears to always point to the backing implementation, not the thunk, so I needed to get the overridden vtable entry from the constant and I think that's reasonable. That gives me:

super_method %10 : $Child, #Base.init!initializer.1 : <T> Base<T>.Type -> (x: T) -> Base<T> , $@convention(method) <τ_0_0> (@in τ_0_0, @owned Base<τ_0_0>) -> @owned Base<τ_0_0>

and

super_method %6 : $Grandchild, #Child.init!initializer.1 : Child.Type -> (x: String) -> Child , $@convention(method) (@in String, @owned Child) -> @owned Child

which look good to me.

With my changes today to fix generic substitutions of partial super methods and getting the right type from the vtable, if I disable that verifier check, devirtualization works correctly with super_method instructions.

Is this a problem with SILDeclRef or is this check simply no longer valid in the verifier? If so, I wonder what the suitable replacement check should be. Maybe something like:

I’d look at the verifier for ClassMethodInst. It turns out that TypeConverter has a getConstantOverrideType that lowers according to the overridden abstraction pattern.

John.