[opt] Devirtualize package class methods more aggressively. #73540
Conversation
| @@ -91,7 +91,8 @@ static bool isEffectivelyFinalMethod(FullApplySite applySite, CanType classType, | |||
|
|
|||
| auto *cmi = cast<MethodInst>(applySite.getCallee()); | |||
|
|
|||
| if (!calleesAreStaticallyKnowable(applySite.getModule(), cmi->getMember())) | |||
| if (!calleesAreStaticallyKnowable(applySite.getModule(), cmi->getMember()) && | |||
| (!cd || cd->getFormalAccess() != AccessLevel::Package)) | |||
There was a problem hiding this comment.
I think getEffectiveAccess() check is more accurate in SIL optimizer. Also don't we want the same for public access? It also can't be subclassed or overriden outside of defining module.
There was a problem hiding this comment.
Why is package access special (i.e different to internal access)?
If the code following this check depends on module.isWholeModule() for internal and public access (see implementation calleesAreStaticallyKnowable) in one of the calls (which I assume might be the case), then why is this not the case for package access level?
| @@ -193,6 +193,22 @@ package func getPkgModuleKlassMemberTBD() -> Int { | |||
| return ModuleTBD.pkgModuleKlassMember() | |||
| } | |||
|
|
|||
| // CHECK-LABEL: sil package @$s4Main23callsPackageClassMethod1cSi6Module0cD0C_tF : $@convention(thin) (@guaranteed PackageClass) -> Int | |||
| // CHECK-NOT: class_method | |||
There was a problem hiding this comment.
This file contains tests in non-resilient mode; could you also add cases to package-cmo-resilient-mode.swift?
| // CHECK-NOT: apply | ||
| // CHECK: return | ||
| public func callsPackageClassMethod(c: PackageClass) -> Int { | ||
| return c.test() |
There was a problem hiding this comment.
Why is it okay to remove the virtual method call?
A function that has a PackageClass argument could be receiving a PackageClass or a PackageClassDerived instance?
There was a problem hiding this comment.
Did we elimate the method dispatch with a conditional branch on the type? (The speculative devirtualizer does stuff like that)
Package class methods cannot be overridden outside of the module, so all callees should be statically knowable, and we can usually devirtualize them.