Skip to content

Commit

Permalink
[sil-devirtualizer] Improve devirtualization of witness_method instru…
Browse files Browse the repository at this point in the history
…ctions.

Handle such cases like partial applications of witness methods and applications of witness methods with substitutions.

Some of these uses-cases occur when there is a protocol defining an operator, a generic struct conforming to this protocol, and the operator conformance of this struct is expressed as a global function.
  • Loading branch information
swiftix committed Dec 1, 2015
1 parent 8885997 commit 5cc14ab
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 17 deletions.
12 changes: 12 additions & 0 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,16 @@ class ApplyInstBase<Impl, Base, false> : public Base {
return {getSubstitutionsStorage(), NumSubstitutions};
}

ArrayRef<Substitution> getSubstitutionsWithoutSelfSubstitution() const {
assert(getNumArguments() && "Should only be called when Callee has "
"at least a self parameter.");
assert(hasSubstitutions() && "Should only be called when Callee has "
"substitutions.");
if (getSubstCalleeType()->hasSelfParam())
return getSubstitutions().slice(1);
return getSubstitutions();
}

/// The arguments passed to this instruction.
MutableArrayRef<Operand> getArgumentOperands() {
return Operands.getDynamicAsArray();
Expand Down Expand Up @@ -4266,6 +4276,8 @@ class ApplySite {
return cast<ApplyInst>(Inst)->getSubstitutionsWithoutSelfSubstitution();
case ValueKind::TryApplyInst:
return cast<TryApplyInst>(Inst)->getSubstitutionsWithoutSelfSubstitution();
case ValueKind::PartialApplyInst:
return cast<PartialApplyInst>(Inst)->getSubstitutionsWithoutSelfSubstitution();
default:
llvm_unreachable("not implemented for this instruction!");
}
Expand Down
3 changes: 2 additions & 1 deletion include/swift/SILPasses/Utils/Devirtualize.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace swift {
/// Two elements are required, because a result of the new devirtualized
/// apply/try_apply instruction (second element) eventually needs to be
/// casted to produce a properly typed value (first element).
typedef std::pair<ValueBase *, FullApplySite> DevirtualizationResult;
typedef std::pair<ValueBase *, ApplySite> DevirtualizationResult;

DevirtualizationResult tryDevirtualizeApply(FullApplySite AI);
bool isClassWithUnboundGenericParameters(SILType C, SILModule &M);
Expand All @@ -49,6 +49,7 @@ DevirtualizationResult devirtualizeClassMethod(FullApplySite AI,
SILValue ClassInstance);
DevirtualizationResult tryDevirtualizeClassMethod(FullApplySite AI,
SILValue ClassInstance);
DevirtualizationResult tryDevirtualizeWitnessMethod(ApplySite AI);
}

#endif
4 changes: 3 additions & 1 deletion lib/IRGen/GenFunc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3591,7 +3591,9 @@ static llvm::Function *emitPartialApplicationForwarder(IRGenModule &IGM,
}

// Emit the polymorphic arguments.
assert(subs.empty() != hasPolymorphicParameters(origType)
assert((subs.empty() != hasPolymorphicParameters(origType) ||
(subs.empty() && origType->getRepresentation() ==
SILFunctionTypeRepresentation::WitnessMethod))
&& "should have substitutions iff original function is generic");
WitnessMetadata witnessMetadata;
if (hasPolymorphicParameters(origType)) {
Expand Down
1 change: 0 additions & 1 deletion lib/IRGen/IRGenSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,6 @@ getPartialApplicationFunction(IRGenSILFunction &IGF,
break;

case SILFunctionTypeRepresentation::WitnessMethod:
assert(false && "partial_apply of witness functions not implemented");

This comment has been minimized.

Copy link
@rjmccall

rjmccall Dec 1, 2015

Contributor

Please just merge this case into the identical case block below.

break;

case SILFunctionTypeRepresentation::Thick:
Expand Down
4 changes: 2 additions & 2 deletions lib/SILPasses/EarlySIL/MandatoryInlining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,14 +320,14 @@ runOnFunctionRecursively(SILFunction *F, FullApplySite AI,
I = II->getIterator();
else
I = NewInst->getParentBB()->begin();
auto NewAI = NewInstPair.second;
auto NewAI = FullApplySite::isa(NewInstPair.second.getInstruction());
if (!NewAI)
continue;

InnerAI = NewAI;
}

SILLocation Loc = InnerAI.getLoc();
SILLocation Loc = InnerAI.getLoc();

This comment has been minimized.

Copy link
@rjmccall

rjmccall Dec 1, 2015

Contributor

Tab.

SILValue CalleeValue = InnerAI.getCallee();
bool IsThick;
PartialApplyInst *PAI;
Expand Down
2 changes: 1 addition & 1 deletion lib/SILPasses/IPO/PerformanceInliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ FullApplySite SILPerformanceInliner::devirtualizeUpdatingCallGraph(
if (!NewInstPair.second)
return FullApplySite();

auto NewAI = NewInstPair.second;
auto NewAI = FullApplySite::isa(NewInstPair.second.getInstruction());
CallGraphEditor(&CG).replaceApplyWithNew(Apply, NewAI);

replaceDeadApply(Apply, NewInstPair.first);
Expand Down
2 changes: 1 addition & 1 deletion lib/SILPasses/SILCombiner/SILCombiner.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class SILCombiner :
SILInstruction *visitAllocRefDynamicInst(AllocRefDynamicInst *ARDI);
SILInstruction *visitEnumInst(EnumInst *EI);
SILInstruction *visitConvertFunctionInst(ConvertFunctionInst *CFI);

SILInstruction *visitWitnessMethodInst(WitnessMethodInst *WMI);

/// Instruction visitor helpers.
SILInstruction *optimizeBuiltinCanBeObjCClass(BuiltinInst *AI);
Expand Down
31 changes: 31 additions & 0 deletions lib/SILPasses/SILCombiner/SILCombinerMiscVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "swift/SILAnalysis/CFG.h"
#include "swift/SILAnalysis/ValueTracking.h"
#include "swift/SILPasses/Utils/Local.h"
#include "swift/SILPasses/Utils/Devirtualize.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/DenseMap.h"
Expand Down Expand Up @@ -1070,3 +1071,33 @@ visitAllocRefDynamicInst(AllocRefDynamicInst *ARDI) {
SILInstruction *SILCombiner::visitEnumInst(EnumInst *EI) {
return nullptr;
}

SILInstruction *SILCombiner::visitWitnessMethodInst(WitnessMethodInst *WMI) {
// Try to devirtualize a witness_method if it is statically possible.
// Many cases are handled by the inliner/devirtualizer, but certain
// special cases are not covered there, e.g. partial_apply(witness_method)
SILFunction *F;
ArrayRef<Substitution> Subs;
SILWitnessTable *WT;

std::tie(F, WT, Subs) =
WMI->getModule().lookUpFunctionInWitnessTable(WMI->getConformance(),
WMI->getMember());

if (!F)
return nullptr;

for (auto U = WMI->use_begin(), E = WMI->use_end(); U != E;) {
auto User = U->getUser();
++U;
if (auto AI = ApplySite::isa(User)) {
auto Result = tryDevirtualizeWitnessMethod(AI);
if (Result.first) {
User->replaceAllUsesWith(Result.first);
eraseInstFromFunction(*User);
}
}
}
return nullptr;
}

25 changes: 15 additions & 10 deletions lib/SILPasses/Utils/Devirtualize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ DevirtualizationResult swift::tryDevirtualizeClassMethod(FullApplySite AI,
/// Generate a new apply of a function_ref to replace an apply of a
/// witness_method when we've determined the actual function we'll end
/// up calling.
static FullApplySite devirtualizeWitnessMethod(FullApplySite AI, SILFunction *F,
static ApplySite devirtualizeWitnessMethod(ApplySite AI, SILFunction *F,
ArrayRef<Substitution> Subs) {
// We know the witness thunk and the corresponding set of substitutions
// required to invoke the protocol method at this point.
Expand All @@ -517,7 +517,10 @@ static FullApplySite devirtualizeWitnessMethod(FullApplySite AI, SILFunction *F,
SmallVector<Substitution, 16> NewSubstList(Subs.begin(), Subs.end());

// Add the non-self-derived substitutions from the original application.
for (auto &origSub : AI.getSubstitutionsWithoutSelfSubstitution())
ArrayRef<Substitution> SubstList;
SubstList = AI.getSubstitutionsWithoutSelfSubstitution();

for (auto &origSub : SubstList)
if (!origSub.getArchetype()->isSelfDerived())
NewSubstList.push_back(origSub);

Expand All @@ -531,18 +534,17 @@ static FullApplySite devirtualizeWitnessMethod(FullApplySite AI, SILFunction *F,
auto Arguments = SmallVector<SILValue, 4>();

auto ParamTypes = SubstCalleeCanType->getParameterSILTypes();
// Type of the current parameter being processed
auto ParamType = ParamTypes.begin();

// Iterate over the non self arguments and add them to the
// new argument list, upcasting when required.
SILBuilderWithScope B(AI.getInstruction());
for (SILValue A : AI.getArguments()) {
if (A.getType() != *ParamType)
A = B.createUpcast(AI.getLoc(), A, *ParamType);
for (unsigned ArgN = 0, ArgE = AI.getNumArguments(); ArgN != ArgE; ++ArgN) {
SILValue A = AI.getArgument(ArgN);
auto ParamType = ParamTypes[ParamTypes.size() - AI.getNumArguments() + ArgN];
if (A.getType() != ParamType)
A = B.createUpcast(AI.getLoc(), A, ParamType);

Arguments.push_back(A);
++ParamType;
}

// Replace old apply instruction by a new apply instruction that invokes
Expand All @@ -553,7 +555,7 @@ static FullApplySite devirtualizeWitnessMethod(FullApplySite AI, SILFunction *F,

auto SubstCalleeSILType = SILType::getPrimitiveObjectType(SubstCalleeCanType);
auto ResultSILType = SubstCalleeCanType->getSILResult();
FullApplySite SAI;
ApplySite SAI;

if (auto *A = dyn_cast<ApplyInst>(AI))
SAI = Builder.createApply(Loc, FRI, SubstCalleeSILType,
Expand All @@ -563,6 +565,9 @@ static FullApplySite devirtualizeWitnessMethod(FullApplySite AI, SILFunction *F,
SAI = Builder.createTryApply(Loc, FRI, SubstCalleeSILType,
NewSubstList, Arguments,
TAI->getNormalBB(), TAI->getErrorBB());
if (auto *PAI = dyn_cast<PartialApplyInst>(AI))
SAI = Builder.createPartialApply(Loc, FRI, SubstCalleeSILType,
NewSubstList, Arguments, PAI->getType());

NumWitnessDevirt++;
return SAI;
Expand All @@ -571,7 +576,7 @@ static FullApplySite devirtualizeWitnessMethod(FullApplySite AI, SILFunction *F,
/// In the cases where we can statically determine the function that
/// we'll call to, replace an apply of a witness_method with an apply
/// of a function_ref, returning the new apply.
static DevirtualizationResult tryDevirtualizeWitnessMethod(FullApplySite AI) {
DevirtualizationResult swift::tryDevirtualizeWitnessMethod(ApplySite AI) {
SILFunction *F;
ArrayRef<Substitution> Subs;
SILWitnessTable *WT;
Expand Down
93 changes: 93 additions & 0 deletions test/SILPasses/devirt_static_witness_method.sil
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// RUN: %target-sil-opt -enable-sil-verify-all %s -sil-combine | FileCheck %s
sil_stage canonical

import Builtin
import Swift
import SwiftShims

protocol CanAdd {
func +(lhs: Self, rhs: Self) -> Self
}

extension Int64 : CanAdd {
}

struct S<T> : CanAdd {
}

func +<T>(lhs: S<T>, rhs: S<T>) -> S<T>


sil hidden [transparent] [thunk] @operator_plus_static_non_generic_witness_for_S : $@convention(thin) <T where T : CanAdd> (@out S<T>, @in S<T>, @in S<T>, @thick S<T>.Type) -> () {
bb0(%0 : $*S<T>, %1 : $*S<T>, %2 : $*S<T>, %3 : $@thick S<T>.Type) :
%17 = tuple ()
return %17 : $()
}

sil hidden [transparent] [thunk] @operator_plus_static_non_generic_witness : $@convention(witness_method) (@out Int64, @in Int64, @in Int64, @thick Int64.Type) -> () {
bb0(%0 : $*Int64, %1 : $*Int64, %2 : $*Int64, %3 : $@thick Int64.Type):
%4 = struct_element_addr %1 : $*Int64, #Int64._value
%5 = load %4 : $*Builtin.Int64
%6 = struct_element_addr %2 : $*Int64, #Int64._value
%7 = load %6 : $*Builtin.Int64
%8 = integer_literal $Builtin.Int1, -1
%9 = builtin "sadd_with_overflow_Int64"(%5 : $Builtin.Int64, %7 : $Builtin.Int64, %8 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
%10 = tuple_extract %9 : $(Builtin.Int64, Builtin.Int1), 0
%11 = tuple_extract %9 : $(Builtin.Int64, Builtin.Int1), 1
cond_fail %11 : $Builtin.Int1
%15 = struct $Int64 (%10 : $Builtin.Int64)
store %15 to %0 : $*Int64
%17 = tuple ()
return %17 : $()
}

// Test that this partial application of a function reference referring to static non generic witness does not crash the IRGen.
// Such code may be produced e.g. when users refer to global operators defined on builtin types.
// CHECK-LABEL: sil hidden @test_partial_apply_of_static_witness
sil hidden @test_partial_apply_of_static_witness : $@convention(thin) () -> @callee_owned (@out Int64, @in Int64, @in Int64) -> () {
bb0:
%1 = metatype $@thick Int64.Type
%2 = function_ref @operator_plus_static_non_generic_witness : $@convention(witness_method) (@out Int64, @in Int64, @in Int64, @thick Int64.Type) -> ()
%3 = partial_apply %2(%1) : $@convention(witness_method) (@out Int64, @in Int64, @in Int64, @thick Int64.Type) -> ()
return %3 : $@callee_owned (@out Int64, @in Int64, @in Int64) -> ()
}

// Test that this partial application of witness_method can be devirtualized.
// CHECK-LABEL: sil hidden @test_devirt_of_partial_apply_of_witness_method
// CHECK-NOT: witness_method $S<Int64>, #CanAdd."+"!1
// CHECK: function_ref @operator_plus_static_non_generic_witness_for_S
// CHECK: partial_apply
// CHECK: return
sil hidden @test_devirt_of_partial_apply_of_witness_method : $@convention(thin) () -> @callee_owned (@out S<Int64>, @in S<Int64>, @in S<Int64>) -> () {
bb0:
%1 = metatype $@thick S<Int64>.Type
%2 = witness_method $S<Int64>, #CanAdd."+"!1 : $@convention(witness_method) <τ_0_0 where τ_0_0 : CanAdd> (@out τ_0_0, @in τ_0_0, @in τ_0_0, @thick τ_0_0.Type) -> ()
%3 = partial_apply %2<S<Int64>>(%1) : $@convention(witness_method) <τ_0_0 where τ_0_0 : CanAdd> (@out τ_0_0, @in τ_0_0, @in τ_0_0, @thick τ_0_0.Type) -> ()
return %3 : $@callee_owned (@out S<Int64>, @in S<Int64>, @in S<Int64>) -> ()
}

// Test that this application of witness_method can be devirtualized.
// CHECK-LABEL: sil hidden @test_devirt_of_apply_of_witness_method
// CHECK-NOT: witness_method $S<Int64>, #CanAdd."+"!1
// CHECK: function_ref @operator_plus_static_non_generic_witness_for_S
// CHECK: apply
// CHECK: return
sil hidden @test_devirt_of_apply_of_witness_method : $@convention(thin) (@in S<Int64>) -> S<Int64> {
bb0(%0 : $*S<Int64>):
%1 = alloc_stack $S<Int64>
%5 = metatype $@thick S<Int64>.Type
%6 = witness_method $S<Int64>, #CanAdd."+"!1 : $@convention(witness_method) <τ_0_0 where τ_0_0 : CanAdd> (@out τ_0_0, @in τ_0_0, @in τ_0_0, @thick τ_0_0.Type) -> ()
%7 = apply %6<S<Int64>>(%1#1, %0, %0, %5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : CanAdd> (@out τ_0_0, @in τ_0_0, @in τ_0_0, @thick τ_0_0.Type) -> ()
%8 = load %1#1: $*S<Int64>
dealloc_stack %1#0 : $*@local_storage S<Int64>
return %8 : $S<Int64>
}

sil_witness_table hidden Int64: CanAdd module Test {
method #CanAdd."+"!1: @operator_plus_static_non_generic_witness
}

sil_witness_table hidden <E where E : CanAdd> S<E>: CanAdd module Test {
method #CanAdd."+"!1: @operator_plus_static_non_generic_witness_for_S
}

3 comments on commit 5cc14ab

@nadavrot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swiftix Roman, in the future please split changes like this into several commits. I would split this commit into:

  1. The IRGen fix
  2. Changes to SILInstruction
  3. Changes to the Devirtualizer and Inliner.
  4. Changes to SILCombine + Testcase.

@swiftix
Copy link
Contributor Author

@swiftix swiftix commented on 5cc14ab Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjmccall John, IRGen changes are based on our discussion. Would you mind reviewing this commit?

@rjmccall
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IRGen changes LGTM; a couple style suggestions.

Please sign in to comment.