Merge pull request #7580 from swiftix/effects-readonly

Do not automatically map Swfit @effects(readonly) to LLVM's readonly attribute

Author: swiftix

lib/IRGen/GenDecl.cpp

@@ -1697,8 +1697,21 @@ static bool isReadOnlyFunction(SILFunction *f) {
     return false;
 
   auto Eff = f->getEffectsKind();
-  return Eff == EffectsKind::ReadNone ||
-         Eff == EffectsKind::ReadOnly;
+
+  // Swift's readonly does not automatically match LLVM's readonly.
+  // Swift SIL optimizer relies on @effects(readonly) to remove e.g.
+  // dead code remaining from initializers of strings or dictionaries
+  // of variables that are not used. But those initializers are often
+  // not really readonly in terms of LLVM IR. For example, the
+  // Dictionary.init() is marked as @effects(readonly) in Swift, but
+  // it does invoke reference-counting operations.
+  if (Eff == EffectsKind::ReadOnly || Eff == EffectsKind::ReadNone) {
+    // TODO: Analyze the body of function f and return true if it is
+    // really readonly.
+    return false;
+  }
+
+  return false;
 }
 
 static clang::GlobalDecl getClangGlobalDeclForFunction(const clang::Decl *decl) {

test/IRGen/readonly.sil

@@ -12,16 +12,32 @@ sil @XXX_ctor : $@convention(method) (@owned XXX) -> @owned XXX
 
 // CHECK: target datalayout
 
-// No @owned or @out parameters -- function is read only at LLVM level
+// No @owned or @out parameters -- function is not read only at LLVM level,
+// because Swift level read only attribute is not automatically mapped
+// to the LLVM read only attribute, unless IRGen can prove by analyzing
+// the function body that this function is really read only.
 
-// CHECK: ; Function Attrs: readonly
-// CHECK-NEXT: define{{( protected)?}} swiftcc void @function_foo(
+// CHECK-NOT: ; Function Attrs: readonly
+// CHECK: define{{( protected)?}} swiftcc void @function_foo(
 sil [readonly] @function_foo : $@convention(thin) (Int) -> () {
 bb0(%0 : $Int):
   %1 = tuple ()
   return %1 : $()
 }
 
+// No @owned or @out parameters -- function is not read only at LLVM level,
+// because Swift level read none attribute is not automatically mapped
+// to the LLVM read only attribute, unless IRGen can prove by analyzing
+// the function body that this function is really read only or read none.
+
+// CHECK-NOT: ; Function Attrs: readonly
+// CHECK: define{{( protected)?}} swiftcc void @function_foo2(
+sil [readnone] @function_foo2 : $@convention(thin) (Int) -> () {
+bb0(%0 : $Int):
+  %1 = tuple ()
+  return %1 : $()
+}
+
 // There's an @owned parameter, and destructors can call arbitrary code
 // -- function is not read only at LLVM level