We discussed this before, but we already break the base ObjC rules since the runtime elides the autorelease message send in MRR when it's possible to perform the retainRV/autoreleaseRV optimization, which only happens when the caller (or a caller higher up in the call-chain if it's tail-called) is compiled with ARC. IIUC, the only semantic change we''ll be making if we emit an objc_autoreleaseReturnValue call instead of an objc_autorelease call is that we no longer guarantee that the message send will happen when the class overrides the autorelease method, which we do guarantee with calls to objc_autorelease (see objc_object::autorelease and objc_object::rootAutorelease in the link below).

In D61808#1505608, @rjmccall wrote:

In D61808#1505544, @ahatanak wrote:

In D61808#1505023, @rjmccall wrote:

The assumption that these methods return the receiver is not true without additional semantic assumptions about these methods, so the current behavior of unconditionally turning [x autorelease] into objc_autorelease(x), x seems quite broken.

Because it's possible to override an autorelease method and make the method return something other than the passed argument, for example?

If it's not possible to assume that the method returns the passed argument in MRR, the direction of this patch and https://reviews.llvm.org/D61970 is completely wrong.

Well, objc_retain and objc_autorelease (and objc_release, trivially) do return the normal return value of the message send in MRR, so assuming that's preserved in the IR, you just need to use that as the expression result. But if the backend's going to unconditionally optimize the intrinsics based on the assumption that those values are the same, that's problematic, just as it would be problematic for it to do ARC-approved reordering on intrinsics emitted for MRR.

In D61808#1505023, @rjmccall wrote:

The assumption that these methods return the receiver is not true without additional semantic assumptions about these methods, so the current behavior of unconditionally turning [x autorelease] into objc_autorelease(x), x seems quite broken.

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

LGTM

Someone reminded me that I could replace the MRR call to objc_autorelease with the ARC call to objc_autoreleaseReturnValue in the front-end. I'm going to try that idea.

Just call EmitCallOrInvoke.

Update the comment in CheckCXXThrowOperand.

Address review comments.

Move the code that was in AlignedExceptionObject.h to getExnObjectAlignment and remove the header.

Define helper function getLinkageTypeForObjCMetadata.

Make sure private linkage is replaced with internal linkage only when the object file format is MachO.

• Instead of passing a flag to CreateMetadataVar that indicates the section is in segment __DATA, just scan the section name string inside CreateMetadataVar.
• Fix test case ns-constant-strings.m.

Address review comments.

In D61147#1479932, @rjmccall wrote:

I would also recommend that you go fix the warning to never fire on virtual C++ methods.

Disable the warning for ObjC methods.

Yes, I guess we can make -Wunused-parameter cover unused parameters of pure @implementation methods too since users can probably just remove them to silence the warning.

In D61147#1479468, @erik.pilkington wrote:

Yeah, I tend to think we should just suppress this unconditionally in Objective-C.

IMO this warning still makes sense for normal functions, or methods that are only declared in an @implementation. Adding a fix-it to cast to void in the function/method body would probably go a long way too.

The test case I'm looking at has a function with over 500k instructions and lots of ObjC runtime function calls.

Fix the comment on why we have to check the size of the basic block before calling DominatorTree::dominate to compute the dominance relation between two instructions in the same basic block.

Diagnose implicitly retained self in ActOnFinishFunctionBody.

Rename function.

Remove a potentially expensive check.

Keep a list of pairs of BlockDecl and SourceLocation and, after the body of an ObjC method is parsed, emit a diagnostic if a SourceLocation in the list is inside an escaping block.

Sorry, clang was failing to diagnose self referenced inside a c++ lambda that was nested inside a block.

LGTM

Just one minor comment, but otherwise the patch looks good to me.