Merge pull request swiftlang#26559 from eeckstein/fix-raw-inst-lowering

Fix property wrappers with tuples.

Author: eeckstein

docs/SIL.rst

@@ -2342,8 +2342,8 @@ assign_by_wrapper
  assign_by_wrapper %0 : $S to %1 : $*T, init %2 : $F, set %3 : $G
  // $S can be a value or address type
  // $T must be the type of a property wrapper.
- // $F must be a function type, taking $S as a single argument and returning $T
- // $G must be a function type, taking $S as a single argument and with not return value
+ // $F must be a function type, taking $S as a single argument (or multiple arguments in case of a tuple) and returning $T
+ // $G must be a function type, taking $S as a single argument (or multiple arguments in case of a tuple) and without a return value
 
 Similar to the ``assign`` instruction, but the assignment is done via a
 delegate.

lib/SIL/SILVerifier.cpp

@@ -1961,6 +1961,34 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
  "Store operand type and dest type mismatch");
  }
 
+ // Usually the assign_by_wrapper initializer or setter has a single argument
+ // (the value). But in case of a tuple, the initializer/setter expect the
+ // tuple elements as separate arguments.
+ void checkAssignByWrapperArgsRecursively(SILType ty,
+ SILFunctionConventions &conv, unsigned &argIdx) {
+ if (auto tupleTy = ty.getAs<TupleType>()) {
+ for (Type et : tupleTy->getElementTypes()) {
+ SILType elTy = SILType::getPrimitiveType(CanType(et), ty.getCategory());
+ checkAssignByWrapperArgsRecursively(elTy, conv, argIdx);
+ }
+ return;
+ }
+ require(argIdx < conv.getNumSILArguments(),
+ "initializer or setter has too few arguments");
+ SILType argTy = conv.getSILArgumentType(argIdx++);
+ if (ty.isAddress() && argTy.isObject())
+ ty = ty.getObjectType();
+ require(ty == argTy,
+ "wrong argument type of initializer or setter");
+ }
+
+ void checkAssignByWrapperArgs(SILType ty, SILFunctionConventions &conv) {
+ unsigned argIdx = conv.getSILArgIndexOfFirstParam();
+ checkAssignByWrapperArgsRecursively(ty, conv, argIdx);
+ require(argIdx == conv.getNumSILArguments(),
+ "initializer or setter has too many arguments");
+ }
+
  void checkAssignByWrapperInst(AssignByWrapperInst *AI) {
  SILValue Src = AI->getSrc(), Dest = AI->getDest();
  require(AI->getModule().getStage() == SILStage::Raw,
@@ -1970,11 +1998,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
  SILValue initFn = AI->getInitializer();
  CanSILFunctionType initTy = initFn->getType().castTo<SILFunctionType>();
  SILFunctionConventions initConv(initTy, AI->getModule());
- unsigned firstArgIdx = initConv.getSILArgIndexOfFirstParam();
- require(initConv.getNumSILArguments() == firstArgIdx + 1,
- "init function has wrong number of arguments");
- require(Src->getType() == initConv.getSILArgumentType(firstArgIdx),
- "wrong argument type of init function");
+ checkAssignByWrapperArgs(Src->getType(), initConv);
  switch (initConv.getNumIndirectSILResults()) {
  case 0:
  require(initConv.getNumDirectSILResults() == 1,
@@ -1998,10 +2022,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
  SILFunctionConventions setterConv(setterTy, AI->getModule());
  require(setterConv.getNumIndirectSILResults() == 0,
  "set function has indirect results");
- require(setterConv.getNumSILArguments() == 1,
- "init function has wrong number of arguments");
- require(Src->getType() == setterConv.getSILArgumentType(0),
- "wrong argument type of init function");
+ checkAssignByWrapperArgs(Src->getType(), setterConv);
  }
 
 #define NEVER_LOADABLE_CHECKED_REF_STORAGE(Name, name, ...) \

lib/SILOptimizer/Mandatory/RawSILInstLowering.cpp

@@ -91,6 +91,80 @@ static void lowerAssignInstruction(SILBuilderWithScope &b, AssignInst *inst) {
  inst->eraseFromParent();
 }
 
+/// Construct the argument list for the assign_by_wrapper initializer or setter.
+///
+/// Usually this is only a single value and a single argument, but in case of
+/// a tuple, the initializer/setter expect the tuple elements as separate
+/// arguments. The purpose of this function is to recursively visit tuple
+/// elements and add them to the argument list \p arg.
+static void getAssignByWrapperArgsRecursively(SmallVectorImpl<SILValue> &args,
+ SILValue src, unsigned &argIdx, const SILFunctionConventions &convention,
+ SILBuilder &forProjections, SILBuilder &forCleanup) {
+
+ SILLocation loc = (*forProjections.getInsertionPoint()).getLoc();
+ SILType srcTy = src->getType();
+ if (auto tupleTy = srcTy.getAs<TupleType>()) {
+ // In case the source is a tuple, we have to destructure the tuple and pass
+ // the tuple elements separately.
+ if (srcTy.isAddress()) {
+ for (unsigned idx = 0, n = tupleTy->getNumElements(); idx < n; ++idx) {
+ auto *TEA = forProjections.createTupleElementAddr(loc, src, idx);
+ getAssignByWrapperArgsRecursively(args, TEA, argIdx, convention,
+ forProjections, forCleanup);
+ }
+ } else {
+ auto *DTI = forProjections.createDestructureTuple(loc, src);
+ for (SILValue elmt : DTI->getAllResults()) {
+ getAssignByWrapperArgsRecursively(args, elmt, argIdx, convention,
+ forProjections, forCleanup);
+ }
+ }
+ return;
+ }
+ assert(argIdx < convention.getNumSILArguments() &&
+ "initializer or setter has too few arguments");
+
+ SILArgumentConvention argConv = convention.getSILArgumentConvention(argIdx);
+ if (srcTy.isAddress() && !argConv.isIndirectConvention()) {
+ // In case of a tuple where one element is loadable, but the other is
+ // address only, we get the whole tuple as address.
+ // For the loadable element, the argument is passed directly, but the
+ // tuple element is in memory. For this case we have to insert a load.
+ src = forProjections.createTrivialLoadOr(loc, src,
+ LoadOwnershipQualifier::Take);
+ }
+ switch (argConv) {
+ case SILArgumentConvention::Indirect_In_Guaranteed:
+ forCleanup.createDestroyAddr(loc, src);
+ break;
+ case SILArgumentConvention::Direct_Guaranteed:
+ forCleanup.createDestroyValue(loc, src);
+ break;
+ case SILArgumentConvention::Direct_Unowned:
+ case SILArgumentConvention::Indirect_In:
+ case SILArgumentConvention::Indirect_In_Constant:
+ case SILArgumentConvention::Direct_Owned:
+ break;
+ case SILArgumentConvention::Indirect_Inout:
+ case SILArgumentConvention::Indirect_InoutAliasable:
+ case SILArgumentConvention::Indirect_Out:
+ case SILArgumentConvention::Direct_Deallocating:
+ llvm_unreachable("wrong convention for setter/initializer src argument");
+ }
+ args.push_back(src);
+ ++argIdx;
+}
+
+static void getAssignByWrapperArgs(SmallVectorImpl<SILValue> &args,
+ SILValue src, const SILFunctionConventions &convention,
+ SILBuilder &forProjections, SILBuilder &forCleanup) {
+ unsigned argIdx = convention.getSILArgIndexOfFirstParam();
+ getAssignByWrapperArgsRecursively(args, src, argIdx, convention,
+ forProjections, forCleanup);
+ assert(argIdx == convention.getNumSILArguments() &&
+ "initializer or setter has too many arguments");
+}
+
 static void lowerAssignByWrapperInstruction(SILBuilderWithScope &b,
  AssignByWrapperInst *inst,
  SmallVectorImpl<BeginAccessInst *> &accessMarkers) {
@@ -103,21 +177,24 @@ static void lowerAssignByWrapperInstruction(SILBuilderWithScope &b,
  SILValue src = inst->getSrc();
  SILValue dest = inst->getDest();
  SILLocation loc = inst->getLoc();
- SILArgumentConvention srcConvention(SILArgumentConvention::Indirect_Inout);
+ SILBuilderWithScope forCleanup(std::next(inst->getIterator()));
 
  switch (inst->getOwnershipQualifier()) {
  case AssignOwnershipQualifier::Init: {
  SILValue initFn = inst->getInitializer();
  CanSILFunctionType fTy = initFn->getType().castTo<SILFunctionType>();
  SILFunctionConventions convention(fTy, inst->getModule());
+ SmallVector<SILValue, 4> args;
  if (convention.hasIndirectSILResults()) {
- b.createApply(loc, initFn, SubstitutionMap(), { dest, src });
- srcConvention = convention.getSILArgumentConvention(1);
+ args.push_back(dest);
+ getAssignByWrapperArgs(args, src, convention, b, forCleanup);
+ b.createApply(loc, initFn, SubstitutionMap(), args);
  } else {
+ getAssignByWrapperArgs(args, src, convention, b, forCleanup);
  SILValue wrappedSrc = b.createApply(loc, initFn, SubstitutionMap(),
- { src });
- b.createTrivialStoreOr(loc, wrappedSrc, dest, StoreOwnershipQualifier::Init);
- srcConvention = convention.getSILArgumentConvention(0);
+ args);
+ b.createTrivialStoreOr(loc, wrappedSrc, dest,
+  StoreOwnershipQualifier::Init);
  }
  // TODO: remove the unused setter function, which usually is a dead
  // partial_apply.
@@ -129,8 +206,9 @@ static void lowerAssignByWrapperInstruction(SILBuilderWithScope &b,
  CanSILFunctionType fTy = setterFn->getType().castTo<SILFunctionType>();
  SILFunctionConventions convention(fTy, inst->getModule());
  assert(!convention.hasIndirectSILResults());
- srcConvention = convention.getSILArgumentConvention(0);
- b.createApply(loc, setterFn, SubstitutionMap(), { src });
+ SmallVector<SILValue, 4> args;
+ getAssignByWrapperArgs(args, src, convention, b, forCleanup);
+ b.createApply(loc, setterFn, SubstitutionMap(), args);
 
  // The destination address is not used. Remove it if it is a dead access
  // marker. This is important, because also the setter function contains
@@ -145,24 +223,6 @@ static void lowerAssignByWrapperInstruction(SILBuilderWithScope &b,
  case AssignOwnershipQualifier::Reinit:
  llvm_unreachable("wrong qualifier for assign_by_wrapper");
  }
- switch (srcConvention) {
- case SILArgumentConvention::Indirect_In_Guaranteed:
- b.createDestroyAddr(loc, src);
- break;
- case SILArgumentConvention::Direct_Guaranteed:
- b.createDestroyValue(loc, src);
- break;
- case SILArgumentConvention::Direct_Unowned:
- case SILArgumentConvention::Indirect_In:
- case SILArgumentConvention::Indirect_In_Constant:
- case SILArgumentConvention::Direct_Owned:
- break;
- case SILArgumentConvention::Indirect_Inout:
- case SILArgumentConvention::Indirect_InoutAliasable:
- case SILArgumentConvention::Indirect_Out:
- case SILArgumentConvention::Direct_Deallocating:
- llvm_unreachable("wrong convention for setter/initializer src argument");
- }
  inst->eraseFromParent();
 }
 

test/SILOptimizer/property_wrappers_and_tuples.swift

@@ -0,0 +1,74 @@
+// RUN: %empty-directory(%t) 
+// RUN: %target-build-swift %s -module-name=a -o %t/a.out
+// RUN: %target-run %t/a.out | %FileCheck %s
+
+// REQUIRES: executable_test
+
+@propertyWrapper struct D<Value> {
+ var wrappedValue: Value {
+ didSet {
+ print("set: \(wrappedValue)")
+ }
+ }
+
+ init(wrappedValue: Value) {
+ self.wrappedValue = wrappedValue
+ print("init: \(wrappedValue)")
+ }
+
+}
+
+struct S<T> {
+ @D var a: (Int,Int)
+ @D var b: (T,T)
+ @D var c: (Int,(Int, Bool))
+ @D var d: (T,Int)
+ @D var e: (Int,(T, Bool))
+
+ init(_ t: T) {
+ // CHECK: init: (27, 28)
+ a = (27, 28)
+ // CHECK: init: (a.X, a.X)
+ b = (t, t)
+ // CHECK: init: (27, (28, true))
+ c = (27, (28, true))
+ // CHECK: init: (a.X, 27)
+ d = (t, 27)
+ // CHECK: init: (27, (a.X, true))
+ e = (27, (t, true))
+ // CHECK: set: (27, 28)
+ a = (27, 28)
+ // CHECK: set: (a.X, a.X)
+ b = (t, t)
+ // CHECK: set: (27, (28, true))
+ c = (27, (28, true))
+ // CHECK: set: (a.X, 27)
+ d = (t, 27)
+ // CHECK: set: (27, (a.X, true))
+ e = (27, (t, true))
+ }
+
+}
+
+class X {
+
+
+ static var numInstances = 0
+
+ init() {
+ X.numInstances += 1
+ }
+
+ deinit {
+ X.numInstances -= 1
+ }
+}
+
+func test() {
+ _ = S(X())
+}
+
+test()
+
+// CHECK: num instances of X: 0
+print("num instances of X: \(X.numInstances)")