[CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR

[ahatanak]

Summary

This patch makes IRGen emit ObjC runtime functions (objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue) that were previously emitted only in ARC mode. This enables retain message sends in MRR code to participate in the retainRV/autoreleaseRV handshake, which keeps returned objects out of the autorelease pool. Also, it enables the ARC optimizer and ARC contract pass to remove retain/autorelease pairs and mark autorelease message sends converted to autoreleaseRV calls as tail calls, which is necessary for the retainRV/autoreleaseRV handshake to succeed.

rdar://problem/50353574

[ddkilzer]

Had a couple questions about using objc_retainAutoreleasedReturnValue without a return statement. (I'm just reading this from a layman's point of view; it may not actually matter in practice.)

test/CodeGenObjC/convert-messages-to-runtime-calls.m

32–33

Silly question: Should objc_retainAutoreleasedReturnValue really be called here when the value is not used in a return statement?

169–170

Same silly question: Should objc_retainAutoreleasedReturnValue really be called here when the value is not used in a return statement?

[ahatanak]

Run the main ARC optimization passes when ObjCNoBuiltinRetainRelease is not set.

test/CodeGenObjC/convert-messages-to-runtime-calls.m

32–33

ARC optimizer has an optimization that converts a call to objc_retainAutoreleasedReturnValue to a call to objc_retain when it doesn't use the result of a call.

http://llvm.org/doxygen/ObjCARCOpts_8cpp_source.html

Also, it converts a call to objc_autoreleaseReturnValue to a call to objc_autorelease when its result isn't used by a return instruction.

[rjmccall]

include/clang/Driver/Options.td

1500

I know I suggested this name, but it should probably be -fno-objc-.... instead of -fobjc-no-.....

Please add help text. Also, there probably ought to be a subsection in the user docs under "Objective-C Features" which talks about message rewriting; it should start by discussing the general fact that we rewrite messages, including how to disable it with -fno-objc-convert-messages-to-runtime-calls, and then segue into the special assumptions for retain/release/autorelease and how to disable them with this option.

Should this be implied by -fno-objc-convert-messages-to-runtime-calls?

lib/CodeGen/CGObjC.cpp

2597

Is using autoreleaseRV in all situations a good idea? Among other things, won't this make it significantly more difficult to detect miscompiles where we fail to tail call autoreleaseRV?

2611

Same question: won't this make it significantly more difficult to detect when we're generating bad code? And the overhead here (on every retain in MRC!) is pretty significant; shouldn't this at least be restricted to situations where it's plausible that it might be reclaiming something, like when the receiver expression is a function call of some sort?

[ahatanak]

lib/CodeGen/CGObjC.cpp

2611

As I mentioned in my response to David's question, I was thinking the ARC optimizer would be able to convert an objc_retainAutoreleasedReturnValue call to an objc_retain call. However, after reading your comment in https://reviews.llvm.org/D61970, I don't think it's okay to use the ARC optimizer to transform code that was compiled in MRR mode.

[ahatanak]

lib/CodeGen/CGObjC.cpp

2611

I meant the comment in https://reviews.llvm.org/D61808.

[ahatanak]

This isn't the right way to make a call to objc_autorelease a tail call, so I'm closing this for now. If we decide to convert calls to objc_autorelease and objc_retain to calls to objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue for other reasons in the future, I can reopen this.

include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(StrictFloatCastOverflow, 1, 1)
 
 CODEGENOPT(UniformWGSize     , 1, 0) ///< -cl-uniform-work-group-size
 CODEGENOPT(NoZeroInitializedInBSS , 1, 0) ///< -fno-zero-initialized-in-bss.
 /// Method of Objective-C dispatch to use.
 ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind, 2, Legacy)
 /// Replace certain message sends with calls to ObjC runtime entrypoints
 CODEGENOPT(ObjCConvertMessagesToRuntimeCalls , 1, 1)
+CODEGENOPT(ObjCNoBuiltinRetainRelease , 1, 0)
 CODEGENOPT(OmitLeafFramePointer , 1, 0) ///< Set when -momit-leaf-frame-pointer is
                                         ///< enabled.
 
 VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
 VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is specified.
 
 /// Choose profile instrumenation kind or no instrumentation.
 ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNone)

include/clang/Driver/Options.td

 def fno_working_directory : Flag<["-"], "fno-working-directory">, Group<f_Group>;
 def fno_wrapv : Flag<["-"], "fno-wrapv">, Group<f_Group>;
 def fno_zero_initialized_in_bss : Flag<["-"], "fno-zero-initialized-in-bss">, Group<f_Group>;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Synthesize retain and release calls for Objective-C pointers">;
 def fno_objc_arc : Flag<["-"], "fno-objc-arc">, Group<f_Group>;
 def fobjc_convert_messages_to_runtime_calls :
   Flag<["-"], "fobjc-convert-messages-to-runtime-calls">, Group<f_Group>;
+def fobjc_no_builtin_retain_release :
+  Flag<["-"], "fobjc-no-builtin-retain-release">, Group<f_Group>,
+  Flags<[CC1Option]>;
 def fno_objc_convert_messages_to_runtime_calls :
   Flag<["-"], "fno-objc-convert-messages-to-runtime-calls">, Group<f_Group>, Flags<[CC1Option]>;
 def fobjc_arc_exceptions : Flag<["-"], "fobjc-arc-exceptions">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Use EH-safe code when synthesizing retains and releases in -fobjc-arc">;
 def fno_objc_arc_exceptions : Flag<["-"], "fno-objc-arc-exceptions">, Group<f_Group>;
 def fobjc_atdefs : Flag<["-"], "fobjc-atdefs">, Group<clang_ignored_f_Group>;
 def fobjc_call_cxx_cdtors : Flag<["-"], "fobjc-call-cxx-cdtors">, Group<clang_ignored_f_Group>;
 def fobjc_exceptions: Flag<["-"], "fobjc-exceptions">, Group<f_Group>,

lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
   if (TM)
     TM->adjustPassManager(PMBuilder);
 
   if (CodeGenOpts.DebugInfoForProfiling ||
       !CodeGenOpts.SampleProfileFile.empty())
     PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
                            addAddDiscriminatorsPass);
 
-  // In ObjC ARC mode, add the main ARC optimization passes.
-  if (LangOpts.ObjCAutoRefCount) {
+  // Add the main ARC optimization passes if ObjC ARC is enabled or
+  // ObjCNoBuiltinRetainRelease isn't set.
+  if (LangOpts.ObjCAutoRefCount || !CodeGenOpts.ObjCNoBuiltinRetainRelease) {
     PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
                            addObjCARCExpandPass);
     PMBuilder.addExtension(PassManagerBuilder::EP_ModuleOptimizerEarly,
                            addObjCARCAPElimPass);
     PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
                            addObjCARCOptPass);
   }
 

lib/CodeGen/CGObjC.cpp

 }
 
 /// Retain the given object which is the result of a function call.
 ///   call i8* \@objc_retainAutoreleasedReturnValue(i8* %value)
 ///
 /// Yes, this function name is one character away from a different
 /// call with completely different semantics.
 llvm::Value *
-CodeGenFunction::EmitARCRetainAutoreleasedReturnValue(llvm::Value *value) {
+CodeGenFunction::EmitARCRetainAutoreleasedReturnValue(llvm::Value *value,
+                                                      llvm::Type *returnType) {
   emitAutoreleasedReturnValueMarker(*this);
   llvm::CallInst::TailCallKind tailKind =
       CGM.getTargetCodeGenInfo()
               .shouldSuppressTailCallsOfRetainAutoreleasedReturnValue()
           ? llvm::CallInst::TCK_NoTail
           : llvm::CallInst::TCK_None;
   return emitARCValueOperation(
-      *this, value, nullptr,
+      *this, value, returnType,
       CGM.getObjCEntrypoints().objc_retainAutoreleasedReturnValue,
       llvm::Intrinsic::objc_retainAutoreleasedReturnValue, tailKind);
 }
 
 /// Claim a possibly-autoreleased return value at +0.  This is only
 /// valid to do in contexts which do not rely on the retain to keep
 /// the object valid for all of its uses; for example, when
 /// the value is ignored, or when it is being assigned to an
llvm::Value *CodeGenFunction::EmitARCAutorelease(llvm::Value *value) {
   return emitARCValueOperation(*this, value, nullptr,
                                CGM.getObjCEntrypoints().objc_autorelease,
                                llvm::Intrinsic::objc_autorelease);
 }
 
 /// Autorelease the given object.
 ///   call i8* \@objc_autoreleaseReturnValue(i8* %value)
 llvm::Value *
-CodeGenFunction::EmitARCAutoreleaseReturnValue(llvm::Value *value) {
-  return emitARCValueOperation(*this, value, nullptr,
+CodeGenFunction::EmitARCAutoreleaseReturnValue(llvm::Value *value,
+                                              llvm::Type *returnType) {
+  return emitARCValueOperation(*this, value, returnType,
                             CGM.getObjCEntrypoints().objc_autoreleaseReturnValue,
                                llvm::Intrinsic::objc_autoreleaseReturnValue,
                                llvm::CallInst::TCK_Tail);
 }
 
 /// Do a fused retain/autorelease of the given object.
 ///   call i8* \@objc_retainAutoreleaseReturnValue(i8* %value)
 llvm::Value *

 
 void CodeGenFunction::emitARCIntrinsicUse(CodeGenFunction &CGF, Address addr,
                                           QualType type) {
   llvm::Value *value = CGF.Builder.CreateLoad(addr);
   CGF.EmitARCIntrinsicUse(value);
 }
 
 /// Autorelease the given object.
-///   call i8* \@objc_autorelease(i8* %value)
+/// If we don't have to handle exceptions and ObjCNoBuiltinRetainRelease isn't
+/// set, call objc_autoreleaseReturnValue. Otherwise, call objc_autorelease.
 llvm::Value *CodeGenFunction::EmitObjCAutorelease(llvm::Value *value,
                                                   llvm::Type *returnType) {
+  // Do not call the ARC intrinsic if we are handling exceptions.
+  if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest())
+    return EmitARCAutoreleaseReturnValue(value, returnType);
   return emitObjCValueOperation(
       *this, value, returnType,
       CGM.getObjCEntrypoints().objc_autoreleaseRuntimeFunction,
       "objc_autorelease");
 }
 
 /// Retain the given object, with normal retain semantics.
-///   call i8* \@objc_retain(i8* %value)
+/// If we don't have to handle exceptions and ObjCNoBuiltinRetainRelease isn't
+/// set, call objc_retainAutoreleasedReturnValue. Otherwise, call objc_retain.
 llvm::Value *CodeGenFunction::EmitObjCRetainNonBlock(llvm::Value *value,
                                                      llvm::Type *returnType) {
+  // Do not call the ARC intrinsic if we are handling exceptions.
+  if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest())
+    return EmitARCRetainAutoreleasedReturnValue(value, returnType);
   return emitObjCValueOperation(
       *this, value, returnType,
       CGM.getObjCEntrypoints().objc_retainRuntimeFunction, "objc_retain");
 }
 
 /// Release the given object.
 ///   call void \@objc_release(i8* %value)
 void CodeGenFunction::EmitObjCRelease(llvm::Value *value,

lib/CodeGen/CodeGenFunction.h

public:
   llvm::Value *EmitARCStoreStrongCall(Address addr, llvm::Value *value,
                                       bool resultIgnored);
   llvm::Value *EmitARCRetain(QualType type, llvm::Value *value);
   llvm::Value *EmitARCRetainNonBlock(llvm::Value *value);
   llvm::Value *EmitARCRetainBlock(llvm::Value *value, bool mandatory);
   void EmitARCDestroyStrong(Address addr, ARCPreciseLifetime_t precise);
   void EmitARCRelease(llvm::Value *value, ARCPreciseLifetime_t precise);
   llvm::Value *EmitARCAutorelease(llvm::Value *value);
-  llvm::Value *EmitARCAutoreleaseReturnValue(llvm::Value *value);
+  llvm::Value *EmitARCAutoreleaseReturnValue(llvm::Value *value,
+                                             llvm::Type *returnType = nullptr);
   llvm::Value *EmitARCRetainAutoreleaseReturnValue(llvm::Value *value);
-  llvm::Value *EmitARCRetainAutoreleasedReturnValue(llvm::Value *value);
+  llvm::Value *
+  EmitARCRetainAutoreleasedReturnValue(llvm::Value *value,
+                                       llvm::Type *returnType = nullptr);
   llvm::Value *EmitARCUnsafeClaimAutoreleasedReturnValue(llvm::Value *value);
 
   llvm::Value *EmitObjCAutorelease(llvm::Value *value, llvm::Type *returnType);
   llvm::Value *EmitObjCRetainNonBlock(llvm::Value *value,
                                       llvm::Type *returnType);
   void EmitObjCRelease(llvm::Value *value, ARCPreciseLifetime_t precise);
 
   std::pair<LValue,llvm::Value*>

lib/Driver/ToolChains/Clang.cpp

static void RenderObjCOptions(const ToolChain &TC, const Driver &D,
 
   // Silence warning for full exception code emission options when explicitly
   // set to use no ARC.
   if (Args.hasArg(options::OPT_fno_objc_arc)) {
     Args.ClaimAllArgs(options::OPT_fobjc_arc_exceptions);
     Args.ClaimAllArgs(options::OPT_fno_objc_arc_exceptions);
   }
 
+  if (Args.hasArg(options::OPT_fobjc_no_builtin_retain_release))
+    CmdArgs.push_back("-fobjc-no-builtin-retain-release");
+
   // Allow the user to control whether messages can be converted to runtime
   // functions.
   if (types::isObjC(Input.getType())) {
     auto *Arg = Args.getLastArg(
         options::OPT_fobjc_convert_messages_to_runtime_calls,
         options::OPT_fno_objc_convert_messages_to_runtime_calls);
     if (Arg &&
         Arg->getOption().matches(

lib/Frontend/CompilerInvocation.cpp

if (Method == ~0U) {
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
       Success = false;
     } else {
       Opts.setObjCDispatchMethod(
         static_cast<CodeGenOptions::ObjCDispatchMethodKind>(Method));
     }
   }
 
+  if (Args.hasArg(OPT_fobjc_no_builtin_retain_release))
+    Opts.ObjCNoBuiltinRetainRelease = 1;
 
   if (Args.hasArg(OPT_fno_objc_convert_messages_to_runtime_calls))
     Opts.ObjCConvertMessagesToRuntimeCalls = 0;
 
   if (Args.getLastArg(OPT_femulated_tls) ||
       Args.getLastArg(OPT_fno_emulated_tls)) {
     Opts.ExplicitEmulatedTLS = true;
     Opts.EmulatedTLS =

test/CodeGenObjC/convert-messages-to-runtime-calls.m

 // RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
-// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
 // RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
 // RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
-// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
 // RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
 // Note: This line below is for tvos for which the driver passes through to use the ios9.0 runtime.
-// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
-// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions -fobjc-no-builtin-retain-release | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-NO-ARC-INTRINSICS
 
 #define nil (id)0
 
 @interface NSObject
 + (id)alloc;
 + (id)allocWithZone:(void*)zone;
 + (id)alloc2;
 - (id)retain;
 - (void)release;
 - (id)autorelease;
 @end
 
 // CHECK-LABEL: define {{.*}}void @test1
 void test1(id x) {
   // MSGS: {{call.*@objc_msgSend}}
   // MSGS: {{call.*@objc_msgSend}}
   // MSGS: {{call.*@objc_msgSend}}
   // MSGS: {{call.*@objc_msgSend}}
   // MSGS: {{call.*@objc_msgSend}}
   // CALLS: {{call.*@objc_alloc}}
   // CALLS: {{call.*@objc_allocWithZone}}
-  // CALLS: {{call.*@objc_retain}}
+  // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+  // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
   // CALLS: {{call.*@objc_release}}
-  // CALLS: {{call.*@objc_autorelease}}
+  // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.autoreleaseReturnValue}}
+  // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_autorelease}}
   [NSObject alloc];
   [NSObject allocWithZone:nil];
   [x retain];
   [x release];
   [x autorelease];
 }
 
 // CHECK-LABEL: define {{.*}}void @check_invoke
void test_alloc_instance(A *a) {
   [a alloc];
   [a allocWithZone:nil];
 }
 
 // Make sure we get a bitcast on the return type as the
 // call will return i8* which we have to cast to A*
 // CHECK-LABEL: define {{.*}}void @test_retain_class_ptr
 A* test_retain_class_ptr(B *b) {
-  // CALLS: {{call.*@objc_retain}}
+  // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+  // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
   // CALLS-NEXT: bitcast i8*
   // CALLS-NEXT: ret
   return [b retain];
 }
 
 // Make sure we get a bitcast on the return type as the
 // call will return i8* which we have to cast to A*
 // CHECK-LABEL: define {{.*}}void @test_autorelease_class_ptr
 A* test_autorelease_class_ptr(B *b) {
-  // CALLS: {{call.*@objc_autorelease}}
+  // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.autoreleaseReturnValue}}
+  // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_autorelease}}
   // CALLS-NEXT: bitcast i8*
   // CALLS-NEXT: ret
   return [b autorelease];
 }
 
 
 @interface C
 + (id)allocWithZone:(int)intArg;

 @end
 
 @implementation NSString
 
 // Make sure we can convert a message to a dynamic receiver to a call
 // CHECK-LABEL: define {{.*}}void @retain_self
 + (void)retain_self {
   // MSGS: {{call.*@objc_msgSend}}
-  // CALLS: {{call.*@objc_retain}}
+  // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+  // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
   [self retain];
 }
 
 // Make sure we never convert a message to super to a call
 // CHECK-LABEL: define {{.*}}void @retain_super
 - (void)retain_super {
   // MSGS: {{call.*@objc_msgSend}}
   // CALLS: {{call.*@objc_msgSend}}