[CodeGen][ObjC] Emit the retainRV marker as a module flag instead of named metadata.

[ahatanak]

Summary

This fixes a bug which causes the ARC contract pass to not insert the assembly marker that is needed for the runtime to do the objc_autoreleaseReturnValue/ objc_retainAutoreleasedReturnValue optimization when compiling on ARM64/ARM with LTO enabled. This happens because IRLinker::linkNamedMDNodes() adds all the operands from the source modules to the metadata's operand list in the merged module (see the example below) and ARC contract fails to extract the marker string if the metadata has more than one operand.

clang.arc.retainAutoreleasedReturnValueMarker = !{!0, !0}
!0 = !{!"mov\09fp, fp\09\09# marker for objc_retainAutoreleaseReturnValue"}

To fix the bug, this patch emits the marker as a module flag.

rdar://problem/49464214

[ahatanak]

Corresponding llvm patch is here: https://reviews.llvm.org/D60303.

[steven_wu]

I talked with Akira offline and we think this is probably the best approach to fix this LTO issue. I will leave others to comment if they think otherwise.

[rjmccall]

Will the ARC-contract pass recognize both, or are you adding a BC upgrader that rewrites the global metadata into a module flag?

[ahatanak]

The upgrader rewrites the named metadata into a module flag (see https://reviews.llvm.org/D60303).

[rjmccall]

Okay. In that case, I have no objections to this.

[ahatanak]

Thanks. Does this require changes to swift too?

[rjmccall]

In D60302#1460379, @ahatanak wrote:

Thanks. Does this require changes to swift too?

Yes, it will, to getObjCRetainAutoreleasedReturnValueMarker in GenObjC.cpp.

cfe/trunk/lib/CodeGen/CGObjC.cpp

// in a moment.
         llvm::FunctionType::get(CGF.VoidTy, /*variadic*/false);
 
       marker = llvm::InlineAsm::get(type, assembly, "", /*sideeffects*/ true);
 
     // If we're at -O1 and above, we don't want to litter the code
     // with this marker yet, so leave a breadcrumb for the ARC
     // optimizer to pick up.
     } else {
-      llvm::NamedMDNode *metadata =
-        CGF.CGM.getModule().getOrInsertNamedMetadata(
-                            "clang.arc.retainAutoreleasedReturnValueMarker");
-      assert(metadata->getNumOperands() <= 1);
-      if (metadata->getNumOperands() == 0) {
-        auto &ctx = CGF.getLLVMContext();
-        metadata->addOperand(llvm::MDNode::get(ctx,
-                                     llvm::MDString::get(ctx, assembly)));
+      const char *markerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
+      if (!CGF.CGM.getModule().getModuleFlag(markerKey)) {
+        auto *str = llvm::MDString::get(CGF.getLLVMContext(), assembly);
+        CGF.CGM.getModule().addModuleFlag(llvm::Module::Error, markerKey, str);
       }
     }
   }
 
   // Call the marker asm if we made one, which we do only at -O0.
   if (marker)
     CGF.Builder.CreateCall(marker, None, CGF.getBundlesForFunclet(marker));
 }

cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m

 // CHECK-NEXT:        [[T2:%.*]] = call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
 // CHECK-NEXT:        bitcast i8* [[T2]] to [[A]]*
 // CHECK-NEXT:        ret void
 
 
 
 // This is always at the end of the module.
 
-// CHECK-OPTIMIZED: !clang.arc.retainAutoreleasedReturnValueMarker = !{!0}
+// CHECK-OPTIMIZED: !llvm.module.flags = !{!0,
+// CHECK-OPTIMIZED: !0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov{{.*}}marker for objc_retainAutoreleaseReturnValue"}