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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: include/clang/Driver/Options.td:1500
+  Flag<["-"], "fobjc-no-builtin-retain-release">, Group<f_Group>,
+  Flags<[CC1Option]>;
 def fno_objc_convert_messages_to_runtime_calls :
----------------
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`?


================
Comment at: lib/CodeGen/CGObjC.cpp:2597
+  if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest())
+    return EmitARCAutoreleaseReturnValue(value, returnType);
   return emitObjCValueOperation(
----------------
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?


================
Comment at: lib/CodeGen/CGObjC.cpp:2611
+  if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest())
+    return EmitARCRetainAutoreleasedReturnValue(value, returnType);
   return emitObjCValueOperation(
----------------
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?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61970/new/

https://reviews.llvm.org/D61970