From cec2b989c12ede7bd8f1d2a0eeece64eb65259e4 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Fri, 1 Feb 2019 14:12:16 -0800 Subject: [PATCH 1/2] [ownership] When transforming destructures => projections, only create projections for used results. This is not technically necessary, but it eliminates some differences in the tests at -Onone. --- .../Transforms/OwnershipModelEliminator.cpp | 27 ++++++++++++++----- .../ownership_model_eliminator.sil | 19 +++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp index c689b79de185d..625ca1bdda06e 100644 --- a/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp +++ b/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp @@ -27,7 +27,9 @@ #include "swift/SIL/SILBuilder.h" #include "swift/SIL/SILFunction.h" #include "swift/SIL/SILVisitor.h" +#include "swift/SILOptimizer/Analysis/SimplifyInstruction.h" #include "swift/SILOptimizer/PassManager/Transforms.h" +#include "swift/SILOptimizer/Utils/Local.h" #include "llvm/Support/CommandLine.h" using namespace swift; @@ -248,6 +250,9 @@ static void splitDestructure(SILBuilder &B, SILInstruction *I, SILValue Op) { assert((isa(I) || isa(I)) && "Only destructure operations can be passed to splitDestructure"); + // First before we destructure anything, see if we can simplify any of our + // instruction operands. + SILModule &M = I->getModule(); SILLocation Loc = I->getLoc(); SILType OpType = Op->getType(); @@ -256,15 +261,23 @@ static void splitDestructure(SILBuilder &B, SILInstruction *I, SILValue Op) { Projection::getFirstLevelProjections(OpType, M, Projections); assert(Projections.size() == I->getNumResults()); - llvm::SmallVector NewValues; - for (unsigned i : indices(Projections)) { - const auto &Proj = Projections[i]; - NewValues.push_back(Proj.createObjectProjection(B, Loc, Op).get()); - assert(NewValues.back()->getType() == I->getResults()[i]->getType() && - "Expected created projections and results to be the same types"); + auto Results = I->getResults(); + for (unsigned Index : indices(Results)) { + SILValue Result = Results[Index]; + + // If our result doesnt have any uses, do not emit instructions, just skip + // it. + if (Result->use_empty()) + continue; + + // Otherwise, create a projection. + const auto &Proj = Projections[Index]; + SingleValueInstruction *ProjInst = + Proj.createObjectProjection(B, Loc, Op).get(); + + Result->replaceAllUsesWith(ProjInst); } - I->replaceAllUsesPairwiseWith(NewValues); I->eraseFromParent(); } diff --git a/test/SILOptimizer/ownership_model_eliminator.sil b/test/SILOptimizer/ownership_model_eliminator.sil index ddfb70b6524da..324ea7e1ade08 100644 --- a/test/SILOptimizer/ownership_model_eliminator.sil +++ b/test/SILOptimizer/ownership_model_eliminator.sil @@ -299,3 +299,22 @@ bb0(%0 : @owned $(Builtin.NativeObject, Builtin.Int32), %1 : @owned $TestArray2) %7 = tuple(%2 : $Builtin.NativeObject, %3 : $Builtin.Int32, %4 : $TestArrayStorage, %5 : $Builtin.Int32, %6 : $TestArrayStorage) return %7 : $(Builtin.NativeObject, Builtin.Int32, TestArrayStorage, Builtin.Int32, TestArrayStorage) } + +// In the following test, the trivial parts do not have any actual uses... do +// not emit any value projections for them! +// +// CHECK-LABEL: sil [canonical] @test_destructure_with_only_some_uses : $@convention(thin) (@owned (Builtin.NativeObject, Builtin.Int32), @owned TestArray2) -> @owned (Builtin.NativeObject, TestArrayStorage, TestArrayStorage) { +// CHECK: bb0([[ARG0:%.*]] : $(Builtin.NativeObject, Builtin.Int32), [[ARG1:%.*]] : $TestArray2): +// CHECK-NEXT: [[ARG0_0:%.*]] = tuple_extract [[ARG0]] +// CHECK-NEXT: [[STORAGE:%.*]] = struct_extract [[ARG1]] : $TestArray2, #TestArray2.storage +// CHECK-NEXT: [[STORAGE2:%.*]] = struct_extract [[ARG1]] : $TestArray2, #TestArray2.storage2 +// CHECK-NEXT: [[RESULT:%.*]] = tuple ([[ARG0_0]] : ${{.*}}, [[STORAGE]] : ${{.*}}, [[STORAGE2]] : ${{.*}}) +// CHECK-NEXT: return [[RESULT]] +// CHECK: } // end sil function 'test_destructure_with_only_some_uses' +sil [canonical] [ossa] @test_destructure_with_only_some_uses : $@convention(thin) (@owned (Builtin.NativeObject, Builtin.Int32), @owned TestArray2) -> @owned (Builtin.NativeObject, TestArrayStorage, TestArrayStorage) { +bb0(%0 : @owned $(Builtin.NativeObject, Builtin.Int32), %1 : @owned $TestArray2): + (%2, %3) = destructure_tuple %0 : $(Builtin.NativeObject, Builtin.Int32) + (%4, %5, %6) = destructure_struct %1 : $TestArray2 + %7 = tuple (%2 : $Builtin.NativeObject, %4 : $TestArrayStorage, %6 : $TestArrayStorage) + return %7 : $(Builtin.NativeObject, TestArrayStorage, TestArrayStorage) +} From 89a447a3dead807dafec0ecb7358566053b6e291 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Sun, 10 Feb 2019 22:27:32 -0800 Subject: [PATCH 2/2] [ownership] When splitting destructures, simplify the instruction if possible. I noticed that this is needed at -Onone to maintain the same codegen output. --- .../Transforms/OwnershipModelEliminator.cpp | 11 ++++++++++- .../ownership_model_eliminator.sil | 11 +++++++++++ test/Serialization/transparent-std.swift | 18 +++++++++++------- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp index 625ca1bdda06e..08fd40fa7ac49 100644 --- a/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp +++ b/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp @@ -275,10 +275,19 @@ static void splitDestructure(SILBuilder &B, SILInstruction *I, SILValue Op) { SingleValueInstruction *ProjInst = Proj.createObjectProjection(B, Loc, Op).get(); + // If we can simplify, do so. + if (SILValue NewV = simplifyInstruction(ProjInst)) { + Result->replaceAllUsesWith(NewV); + ProjInst->eraseFromParent(); + continue; + } + Result->replaceAllUsesWith(ProjInst); } - I->eraseFromParent(); + // We may have exposed trivially dead instructions due to + // simplifyInstruction... delete I and any such instructions! + recursivelyDeleteTriviallyDeadInstructions(I, true); } bool OwnershipModelEliminatorVisitor::visitDestructureStructInst( diff --git a/test/SILOptimizer/ownership_model_eliminator.sil b/test/SILOptimizer/ownership_model_eliminator.sil index 324ea7e1ade08..23c9a34854690 100644 --- a/test/SILOptimizer/ownership_model_eliminator.sil +++ b/test/SILOptimizer/ownership_model_eliminator.sil @@ -318,3 +318,14 @@ bb0(%0 : @owned $(Builtin.NativeObject, Builtin.Int32), %1 : @owned $TestArray2) %7 = tuple (%2 : $Builtin.NativeObject, %4 : $TestArrayStorage, %6 : $TestArrayStorage) return %7 : $(Builtin.NativeObject, TestArrayStorage, TestArrayStorage) } + +// CHECK-LABEL: sil [canonical] @test_simplify_instruction : $@convention(thin) (@owned Builtin.NativeObject, Builtin.Int32) -> @owned Builtin.NativeObject { +// CHECK: bb0([[ARG:%.*]] : $Builtin.NativeObject, +// CHECK-NEXT: return [[ARG]] +// CHECK: } // end sil function 'test_simplify_instruction' +sil [canonical] [ossa] @test_simplify_instruction : $@convention(thin) (@owned Builtin.NativeObject, Builtin.Int32) -> @owned Builtin.NativeObject { +bb0(%0 : @owned $Builtin.NativeObject, %1 : $Builtin.Int32): + %2 = tuple(%0 : $Builtin.NativeObject, %1 : $Builtin.Int32) + (%3, %4) = destructure_tuple %2 : $(Builtin.NativeObject, Builtin.Int32) + return %3 : $Builtin.NativeObject +} diff --git a/test/Serialization/transparent-std.swift b/test/Serialization/transparent-std.swift index a5870f1e4c3e1..4016c336250f2 100644 --- a/test/Serialization/transparent-std.swift +++ b/test/Serialization/transparent-std.swift @@ -16,13 +16,17 @@ func test_foo(x: Builtin.Int1, y: Builtin.Int1) -> Builtin.Int1 { } // SIL-LABEL: sil public_external [transparent] [serialized] [canonical] @$s19def_transparent_std12assign_tuple1x1yyBi64__Bot_BptF : $@convention(thin) (Builtin.Int64, @guaranteed Builtin.NativeObject, Builtin.RawPointer) -> () { -// SIL: = tuple (%0 : $Builtin.Int64, %1 : $Builtin.NativeObject) -// SIL: retain_value -// SIL: = tuple_extract -// SIL: = tuple_extract -// SIL: = pointer_to_address -// SIL: = tuple -// SIL: = load +// SIL: bb0([[ARG0:%.*]] : ${{.*}}, [[ARG1:%.*]] : ${{.*}}, [[ARG2:%.*]] : +// SIL: [[TUP:%.*]] = tuple ([[ARG0]] : $Builtin.Int64, [[ARG1]] : $Builtin.NativeObject) +// SIL: retain_value [[TUP]] +// SIL: [[CASTED_PTR:%.*]] = pointer_to_address [[ARG2]] +// SIL: [[CASTED_PTR_0:%.*]] = tuple_element_addr [[CASTED_PTR]] : $*(Builtin.Int64, Builtin.NativeObject), 0 +// SIL: store [[ARG0]] to [[CASTED_PTR_0]] +// SIL: [[CASTED_PTR_1:%.*]] = tuple_element_addr [[CASTED_PTR]] : $*(Builtin.Int64, Builtin.NativeObject), 1 +// SIL: [[OLD_VALUE:%.*]] = load [[CASTED_PTR_1]] +// SIL: store [[ARG1]] to [[CASTED_PTR_1]] +// SIL: strong_release [[OLD_VALUE]] +// SIL: } // end sil function '$s19def_transparent_std12assign_tuple1x1yyBi64__Bot_BptF' func test_tuple(x: (Builtin.Int64, Builtin.NativeObject), y: Builtin.RawPointer) { assign_tuple(x: x, y: y)