Merge pull request #70444 from eeckstein/deinit-devirt-part2

deinit-devirtualization: completely de-compose non-copyable destroys

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift

@@ -30,7 +30,7 @@ func devirtualizeDeinits(of destroy: DestroyAddrInst, _ context: some MutatingCo
 
 private func devirtualize(destroy: some DevirtualizableDestroy, _ context: some MutatingContext) -> Bool {
   let type = destroy.type
-  guard type.isMoveOnly && type.selfOrAnyFieldHasValueDeinit(in: destroy.parentFunction) else {
+  if !type.isMoveOnly {
     return true
   }
   precondition(type.isNominal, "non-copyable non-nominal types not supported, yet")
@@ -47,9 +47,12 @@ private func devirtualize(destroy: some DevirtualizableDestroy, _ context: some
     // the struct fields or enum cases.
     if type.isStruct {
       result = destroy.devirtualizeStructFields(context)
-    } else {
-      precondition(type.isEnum, "unknown nominal value type")
+    } else if type.isEnum {
       result = destroy.devirtualizeEnumPayloads(context)
+    } else {
+      precondition(type.isClass, "unknown non-copyable type")
+      // A class reference cannot be further de-composed.
+      return true
     }
   }
   context.erase(instruction: destroy)

SwiftCompilerSources/Sources/SIL/Type.swift

@@ -43,10 +43,6 @@ public struct Type : CustomStringConvertible, NoReflectionChildren {
     return bridged.isReferenceCounted(function.bridged)
   }
 
-  public func selfOrAnyFieldHasValueDeinit(in function: Function) -> Bool {
-    return bridged.selfOrAnyFieldHasValueDeinit(function.bridged)
-  }
-
   public var isUnownedStorageType: Bool {
     return bridged.isUnownedStorageType()
   }

include/swift/SIL/SILBridging.h

@@ -115,7 +115,6 @@ struct BridgedType {
   BRIDGED_INLINE bool isValueTypeWithDeinit() const;
   BRIDGED_INLINE bool isLoadable(BridgedFunction f) const;
   BRIDGED_INLINE bool isReferenceCounted(BridgedFunction f) const;
-  BRIDGED_INLINE bool selfOrAnyFieldHasValueDeinit(BridgedFunction f) const;
   BRIDGED_INLINE bool isUnownedStorageType() const;
   BRIDGED_INLINE bool hasArchetype() const;
   BRIDGED_INLINE bool isNominalOrBoundGenericNominal() const;

include/swift/SIL/SILBridgingImpl.h

@@ -88,12 +88,6 @@ bool BridgedType::isReferenceCounted(BridgedFunction f) const {
   return unbridged().isReferenceCounted(f.getFunction());
 }
 
-bool BridgedType::selfOrAnyFieldHasValueDeinit(BridgedFunction f) const {
-  swift::SILType contextType = unbridged().hasTypeParameter() ? f.getFunction()->mapTypeIntoContext(unbridged())
-                                                              : unbridged();
-  return f.getFunction()->getTypeLowering(contextType).selfOrAnyFieldHasValueDeinit();
-}
-
 bool BridgedType::isUnownedStorageType() const {
   return unbridged().isUnownedStorageType();
 }

include/swift/SIL/TypeLowering.h

@@ -192,11 +192,10 @@ class TypeLowering {
       InfiniteFlag               = 1 << 5,
       HasRawPointerFlag          = 1 << 6,
       LexicalFlag                = 1 << 7,
-      HasValueDeinitFlag         = 1 << 8,
     };
     // clang-format on
 
-    uint16_t Flags;
+    uint8_t Flags;
   public:
     /// Construct a default RecursiveProperties, which corresponds to
     /// a trivial, loadable, fixed-layout type.
@@ -273,9 +272,6 @@ class TypeLowering {
     IsLexical_t isLexical() const {
       return IsLexical_t((Flags & LexicalFlag) != 0);
     }
-    bool hasValueDeinit() const {
-      return (Flags & HasValueDeinitFlag) != 0;
-    }
 
     void setNonTrivial() { Flags |= NonTrivialFlag; }
     void setNonFixedABI() { Flags |= NonFixedABIFlag; }
@@ -289,7 +285,6 @@ class TypeLowering {
     void setLexical(IsLexical_t isLexical) {
       Flags = (Flags & ~LexicalFlag) | (isLexical ? LexicalFlag : 0);
     }
-    void setHasValueDeinit() { Flags |= HasValueDeinitFlag; }
   };
 
 private:
@@ -368,10 +363,6 @@ class TypeLowering {
     return Properties.isOrContainsRawPointer();
   }
 
-  bool selfOrAnyFieldHasValueDeinit() const {
-    return Properties.hasValueDeinit();
-  }
-
   /// Returns true if the type is a scalar reference-counted reference, which
   /// can be retained and released.
   bool isReferenceCounted() const {

lib/SIL/IR/TypeLowering.cpp

@@ -2369,9 +2369,6 @@ namespace {
         properties.setLexical(IsLexical);
         return handleMoveOnlyAddressOnly(structType, properties);
       }
-      if (D->getValueTypeDestructor()) {
-        properties.setHasValueDeinit();
-      }
 
       auto subMap = structType->getContextSubstitutionMap(&TC.M, D);
 
@@ -2454,9 +2451,6 @@ namespace {
         return new (TC) LoadableEnumTypeLowering(enumType, properties,
                                                  Expansion);
       }
-      if (D->getValueTypeDestructor()) {
-        properties.setHasValueDeinit();
-      }
 
       auto subMap = enumType->getContextSubstitutionMap(&TC.M, D);
 

test/SILOptimizer/devirt_deinits.sil

@@ -47,6 +47,11 @@ import SwiftShims
   deinit
 }
 
+@_moveOnly struct S5 {
+  @_hasStorage let a: Int
+  @_hasStorage let b: AnyObject
+}
+
 // CHECK-LABEL: sil [ossa] @test_simple_struct :
 // CHECK:         [[D:%.*]] = function_ref @s1_deinit
 // CHECK:         apply [[D]](%0) : $@convention(method) (@owned S1) -> ()
@@ -72,6 +77,17 @@ bb0(%0 : @owned $StrWithoutDeinit):
   return %r : $()
 }
 
+// CHECK-LABEL: sil [ossa] @test_no_deinit :
+// CHECK:         ({{%.*}}, [[S2:%.*]]) = destructure_struct %0
+// CHECK-NEXT:    destroy_value [[S2]]
+// CHECK-NEXT:    tuple ()
+// CHECK:       } // end sil function 'test_no_deinit'
+sil [ossa] @test_no_deinit : $@convention(thin) (@owned S5) -> () {
+bb0(%0 : @owned $S5):
+  destroy_value %0 : $S5
+  %r = tuple()
+  return %r : $()
+}
 // CHECK-LABEL: sil [ossa] @test_indirect_deinit_arg :
 // CHECK:         [[D:%.*]] = function_ref @s3_deinit
 // CHECK:         [[S:%.*]] = alloc_stack $S3<Int>

test/SILOptimizer/moveonly_deinit_devirtualization.sil

@@ -284,7 +284,9 @@ bb2(%4 : $Builtin.Int32):
 
 
 bb3(%6 : @owned $(Klass, Klass)):
-  destroy_value %6 : $(Klass, Klass)
+  (%7, %8) = destructure_tuple %6 : $(Klass, Klass)
+  destroy_value %7 : $Klass
+  destroy_value %8 : $Klass
   br bb6
 
 bb4(%9 : $(Builtin.Int64, Builtin.Int64)):

test/SILOptimizer/moveonly_deinit_devirtualization_library_evolution.sil

@@ -285,7 +285,10 @@ bb2:
 
 bb3:
   %6 = unchecked_take_enum_data_addr %0 : $*NonTrivialMoveOnlyEnum, #NonTrivialMoveOnlyEnum.third
-  destroy_addr %6 : $*(Klass, Klass)
+  %7 = tuple_element_addr %6 : $*(Klass, Klass), 0
+  %8 = tuple_element_addr %6 : $*(Klass, Klass), 1
+  destroy_addr %7 : $*Klass
+  destroy_addr %8 : $*Klass
   br bb6
 
 bb4:

test/SILOptimizer/performance-annotations.swift

@@ -471,3 +471,12 @@ func useOfExistentialNoRuntime() -> P {
   Str(x: 1) // expected-error {{Using type 'any P' can cause metadata allocation or locks}}
 }
 
+public struct NonCopyable: ~Copyable {
+  var value: Int
+}
+
+@_noAllocation
+public func testNonCopyable(_ foo: consuming NonCopyable) {
+  let _ = foo.value
+}
+