[ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

[ahatanak]

Summary

Background:

This fixes a longstanding problem where llvm breaks ARC's autorelease optimization (see the link below) by separating calls from the marker instructions or retainRV/claimRV calls. The backend changes are in https://reviews.llvm.org/D92569.

https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-autoreleasereturnvalue

What this patch does to fix the problem:

• The front-end adds operand bundle "clang.arc.attachedcall" to calls which indicates the call is implicitly followed by a marker instruction and an implicit retainRV/claimRV call that consumes the call result. In addition, it emits a call to @llvm.objc.clang.arc.noop.use, which consumes the call result, to prevent the middle-end passes from changing the return type of the called function. This is currently done only when the target is arm64 and the optimization level is higher than -O0.

• ARC optimizer temporarily emits retainRV/claimRV calls after the calls with the operand bundle in the IR and removes the inserted calls after processing the function.

• ARC contract pass emits retainRV/claimRV calls after the call with the operand bundle. It doesn't remove the operand bundle on the call since the backend needs it to emit the marker instruction. The retainRV/claimRV calls are emitted late in the pipeline to prevent optimization passes from transforming the IR in a way that makes it harder for the ARC middle-end passes to figure out the def-use relationship between the call and the retainRV/claimRV calls (which is the cause of PR31925).

• The function inliner removes an autoreleaseRV call in the callee if nothing in the callee prevents it from being paired up with the retainRV/claimRV call in the caller. It then inserts a release call if claimRV is attached to the call since autoreleaseRV+claimRV is equivalent to a release. If it cannot find an autoreleaseRV call, it tries to transfer the operand bundle to a function call in the callee. This is important since the ARC optimizer (ObjCARCOpt::OptimizeReturns) can remove the autoreleaseRV returning the callee result, which makes it impossible to pair it up with the retainRV/claimRV call in the caller. If that fails, it simply emits a retain call in the IR if retainRV is attached to the call and does nothing if claimRV is attached to it.

• SCCP refrains from replacing the return value of a call with a constant value if the call has the operand bundle. This ensures the call always has at least one user (the call to @llvm.objc.clang.arc.noop.use), which is necessary to prevent passes like deadargelim from changing the return type.

• This patch also fixes a bug in replaceUsesOfNonProtoConstant where multiple operand bundles of the same kind were being added to a call.

Future work:

• Use the operand bundle on x86-64.

• Fix the auto upgrader to convert call+retainRV/claimRV pairs into calls with the operand bundles.

rdar://71443534

[rjmccall]

clang/lib/CodeGen/CGObjC.cpp

2283

This is too generic for a global variable name.

It should also be const.

2329

It would be good to explain why this is target-specific in a comment.

2335

Didn't we just add this? Can that fail to add the marker?

[ahatanak]

clang/lib/CodeGen/CGObjC.cpp

2329

I didn't really have a reason for limiting this to arm64 and non-windows OS other than I didn't want to do too many things in the initial patch. I don't think it would be hard to do this on x86-64. For windows, it looks like we'll have to emit the markers and the ARC calls before CodeGen (before WinEHPrepare) is run, but I haven't thought it through.

2335

The check for module flag was here because the initial version of this patch added the attribute when the target wasn't arm64 (e.g., x86-64) too. I've removed the check for now.

[compnerd]

clang/lib/CodeGen/CGObjC.cpp

2327

Nit: I think that > 0 is easier to read.

2329

Hmm, why the explicit check for not-Windows?

llvm/include/llvm/Analysis/ObjCARCRVAttr.h

44

Nit: A new line before this would be nice.

llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp

443

It seems that IsBundled is unused after this point, why not just do:

if (BundledInsts->contains(Inst))
  return false;

[ahatanak]

Address review comment.

clang/lib/CodeGen/CGObjC.cpp

2329

My understanding is that we'll have to do things differently in ARC contract on Windows, so for now, this patch disables this optimization on Windows.

Is the FIXME comment about Windows I added correct?.

[compnerd]

There is nothing particularly special about that. The reason for the funclet handling there is that in the case of an outlined block for exception handling (i.e. a funclet), we need to ensure that the assembly marker received the funclet token as failure to do so would mark the assembly as unreachable and would thus thwart the auto-release claim. For the normal codepath, the behaviour should be identical.

[ahatanak]

Fix an incorrect comment in insertRetainOrClaimRVCalls in InlineFunction.cpp.

[ahatanak]

In D92808#2443006, @compnerd wrote:

There is nothing particularly special about that. The reason for the funclet handling there is that in the case of an outlined block for exception handling (i.e. a funclet), we need to ensure that the assembly marker received the funclet token as failure to do so would mark the assembly as unreachable and would thus thwart the auto-release claim. For the normal codepath, the behaviour should be identical.

ARC contract pass removes attribute retainRV/claimRV (but not attribute rv_marker) from a call and inserts a retainRV/claimRV call after the call in the IR. In that case, the inserted retainRV/claimRV should get the funclet token. Is that right?

[ahatanak]

Hmm, it seems that calls to some of the ARC runtime functions don't get the token. In some cases, I'm seeing WinEHPrepare (ToT clang, without this patch) turn a call to @objc_unsafeClaimAutoreleasedReturnValue in an EH cleanup block into an unreachable.

[ahatanak]

Enable the optimization on Windows.

Move createCallInst defined in ObjCARCContract.cpp to ObjCARC.h and use it to create calls to retainRV/claimRV with funclet tokens when ARC contraction is run. Add a test case to contract-marker-funclet.ll.

[ahatanak]

ToT clang/llvm (without this patch) is turning the call to @objc_unsafeClaimAutoreleasedReturnValue in the EH cleanup block into an unreachable when compiling the following code. It looks like this happens because the inliner isn't adding the funclet token, but it's not clear to me whether this is a bug or not.

$ cat test.mm

void foo();
id noexcept_func() noexcept;

struct S0 {
  id x;
  ~S0() {
    noexcept_func();
  }
};

void test_attr_claimRV() {
  S0 s0;
  foo();
}

$ clang++ -std=c++11 -fobjc-arc test.mm -O3 -S -o - "-fobjc-runtime=ios-11.0.0" -target aarch64-unknown-windows-msvc

[ahatanak]

Elaborate on when createCallInstWithColors should be called.

[compnerd]

Hmm, I think that the function is marked as noexcept, so the the funclet shouldn't be invoked and should get DCE'd, but that seems unrelated to ARC.

[ahatanak]

Rebase.

[ahatanak]

Set the insert point before generating new instructions in insertRetainOrClaimRVCalls.

[ahatanak]

Rebase and fix a few bugs in the patch.

• In the loop in DeadArgumentEliminationPass::SurveyFunction that visits the uses of a function, continue instead of break so that the function is marked live when a function is used by something that isn't a CallBase.

• Make ObjCARCOpt::OptimizeIndividualCallImpl return early if the instruction is a retainRV/claimRV that was temporarily inserted after a call annotated with a retainRV/claimRV attribute. This fixes the first assertion in ObjCARCOpt::OptimizeRetainRVCall.

[ahatanak]

ping

[rjmccall]

I think the plan should be to use this code pattern on all targets, but starting it with arm64 seems fine.

clang/lib/CodeGen/CGObjC.cpp

2341

It's weird that these attributes use two different capitalization styles. Also, why are they both needed? Did you consider making the role (retain/claim) be the value of the attribute rather than a separate attribute?

Should the attribute be namespaced, like clang.arc.rv_marker?

Let's go ahead and add globals for these strings so we can refer to them symbolically, like you did with retainRVMarkerKey. Is there an LLVM header for ARC optimization we could reasonably pull them from, or are we doomed to repeat ourselves across projects?

[ahatanak]

Add helper functions to ObjCARCRVAttr.h for adding, removing, or looking up attributes.

[ahatanak]

Update patch.

[rjmccall]

Okay, so it's important that we emit both attributes? I'll take your word for it.

llvm/include/llvm/Analysis/ObjCARCRVAttr.h

57

This is just doing the lookup twice, right? You can just unconditionally remove the attribute, here and below.

[ahatanak]

clang/lib/CodeGen/CGObjC.cpp

2341

I think you are asking why both retain/claim and rv_marker are needed? Or are you asking why both 'retain' and 'claim' are needed?

We could do without clang.arc.rv_marker and just use clang.arc.rv=retain/claim since the middle-end and backend passes can easily determine whether a marker is needed or not. The only drawback to using only clang.arc.rv I can think of is that it's no longer possible to tell whether an instruction is implicitly followed by marker+retain/claimRV or just the marker, which makes Instruction::mayWriteToMemory return a conservative answer if the function call is read-only. A read-only call cannot be treated as read-only if it's followed by marker+retain/claimRV, but it is still read-only if it is followed just by the marker.

Note that ARC contract pass emits the retain/claimRV instruction into the IR and drops the corresponding clang.arc.rv attribute but doesn't remove the clang.arc.rv_marker attribute. So if we used only clang.arc.rv, a call annotated with the attribute would be implicitly followed by marker+retain/claimRV before ARC contract, while it would be followed by just the marker after ARC contract, but Instruction::mayWriteToMemory wouldn't' be able to tell the difference.

[rjmccall]

clang/lib/CodeGen/CGObjC.cpp

2341

I think you are asking why both retain/claim and rv_marker are needed?

Yes.

We could do without clang.arc.rv_marker and just use clang.arc.rv=retain/claim since the middle-end and backend passes can easily determine whether a marker is needed or not. The only drawback to using only clang.arc.rv I can think of is that it's no longer possible to tell whether an instruction is implicitly followed by marker+retain/claimRV or just the marker, which makes Instruction::mayWriteToMemory return a conservative answer if the function call is read-only. A read-only call cannot be treated as read-only if it's followed by marker+retain/claimRV, but it is still read-only if it is followed just by the marker.

Is there a situation where we emit the marker but not a following call? Or is that just in the places where we've already made the following call explicit in IR? Because in the latter case, presumably the following call will still act as a barrier to anything that's querying mayWriteToMemory, right? (That is: in general, I expect that nobody just asks whether a specific instruction can write to memory, they ask if any of the instructions in a region can write to memory, and since the following call can...)

[ahatanak]

clang/lib/CodeGen/CGObjC.cpp

2341

I think you are right. The retainRV/claimRV instruction that follows the annotated call will act as a barrier, so using clang.arc.rv_marker to distinguish between marker+retain/claimRV and marker only probably won't help the optimizations in practice.

[ahatanak]

Stop using clang.arc.rv_marker and just use "clang.arc.rv"="retain/claim".

[rjmccall]

LGTM. I know you mentioned doing this for more targets in follow-up commits. It would be good to also do this for -O0, so that we have a single correct pattern coming out of the frontend in all modes; but that's also okay to do in follow-ups.

[nikic]

llvm/lib/IR/Instruction.cpp

580

This change looks pretty fishy. Objective C shouldn't be hijacking LLVMs core instruction model in this way. If it writes to memory, this should either be reflected in the attributes, or modeled using operand bundles.

@fhahn Did you review these changes? If not, I'd suggest to revert this patch and get a review on the LLVM changes.

[rsmith]

This change violates layering by including an Analysis header from IR; I'll be landing a revert as soon as I've finished testing it.

[rjmccall]

In D92808#2521042, @rsmith wrote:

This change violates layering by including an Analysis header from IR; I'll be landing a revert as soon as I've finished testing it.

Should the header just be in IR? We'd like to avoid duplicating constant strings unnecessarily between IRGen and IR passes.

llvm/lib/IR/Instruction.cpp

580

This could definitely be an operand bundle, and I suppose the presence of a bundle does force a conservative assumption on passes.

[rsmith]

Reverted in rG925ae8c790c7e354f12ec14a6cac6aa49fc75b29 for now.

[fhahn]

llvm/include/llvm/Analysis/ObjCARCRVAttr.h

28

Is there a place the attributes could be documented?

llvm/lib/IR/Instruction.cpp

580

@fhahn Did you review these changes?

Nope I didn't have time to look at this so far.

<snip>

Can functions marked as readonly/readnone be called with the objc attributes?

I'm not very familiar with ObjC, but even for a function that just returns a passed in object id, don't we need to retain & release the object in the function? Which means the function cannot be readonly because we need to call @llvm.objc* functions? If that's the case, could we just check in the verifier that the attributes are never used with readonly functions?

If there are indeed cases where we can call readonly functions, operand bundles would probably be safest. It would probably also good to have at least a few alias-analysis tests, to make sure things work as expected.

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

576

This seems fragile. IIUC this workaround is needed because the return value of a retainRV/claimRV function always has one implicit use. This aspect could get missed in other places as well.

As an alternative, could the frontend insert 'dummy' uses (e.g. llvm.objc.arc.use) to make this assumption transparent? Those could be removed sufficiently late. Their placement doesn't really matter and neither does if any instructions are moved between the original call and the dummy use, but the fact that the returned value is used is now explicit. IICU they could be marked as accessing inaccessible memory only, so they should be too much trouble for other optimizations (at least not more than the previous explicit release calls). Or perhaps there's a different way to mark the return value as used explicitly.

[ahatanak]

llvm/lib/IR/Instruction.cpp

580

A function compiled using ARC can call a function compiled using MRR, which can be readonly/readnone. Also, a function compiled using ARC can be marked as readonly/readnone (if an optimization pass wants to do so) after ARC optimizer removes the retainRV/autoreleaseRV pair.

define i8* @foo() {
  %1 = tail call i8* @readonlyMRRFunc()
  ; This function can be readonly if ARC optimizer removes the following two instructions.
  %2 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %1)
  %3 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %2)
  ret i8* 
}

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

576

I think we can make the front-end insert a dummy use instruction. The front-end currently emits @llvm.objc.clang.arc.use in some cases to make sure the object doesn't get released too early, but we can't use that in this case since ARC optimizer treats a call to @llvm.objc.clang.arc.use as a real use, so we probably need a new intrinsic.

[fhahn]

llvm/lib/IR/Instruction.cpp

580

A function compiled using ARC can call a function compiled using MRR, which can be readonly/readnone

Ok, sounds like a bundle would be a good option then?

[ahatanak]

llvm/lib/IR/Instruction.cpp

580

Yes. I think bundle should be used here.

I'm considering passing an integer flag, which distinguishes between retain and claim (e.g., 0 for retain and 1 for claim), to the bundles:

call i8* @foo() [ "clang.arc.rv"(i64 0) ]

Do you see any problems with this approach? Alternatively, we could pass the pointer to the runtime function (e.g., pointer to @llvm.objc.retainAutoreleasedReturnValue).

[ahatanak]

Address post-commit review comments.

• Use operand bundle instead of attribute.
• Emit a call to @llvm.objc.clang.arc.noop.use in the front-end so that the optimization passes know the call result is used.

[ahatanak]

Teach markTails to ignore operand bundle clang.arc.rv when determining whether a call can be marked as tail. ObjCARCOpt::OptimizeReturn cannot eliminate the retainRV/autoreleaseRV pair if the call isn't marked as tail.

[fhahn]

Thanks for the update! The approach with operand bundles and the no-op uses looks cleaner than the original version. The special handling in the inliner seems a bit unfortunate, but I guess there's no way around that?

(as a side note, it might be easier to submit if this would be split up in the changes adding the bundle, followed by the ARC optimization & clang changes)

llvm/include/llvm/Analysis/ObjCARCUtil.h

41 ↗

(On Diff #320001)

I think we should include LLVMContext.h explicitly?

llvm/include/llvm/IR/InstrTypes.h

1217 ↗

(On Diff #320001)

Please add documentation for the new functions.

llvm/include/llvm/IR/Intrinsics.td

449 ↗

(On Diff #320001)

Can this be a DefaultAttrIntrinsics (which adds nofree nosync nounwind willreturn)?

Same probably for all/most objc_* intrinsics.

llvm/lib/Transforms/Utils/InlineFunction.cpp

1650

Could you add a comment elaborating what this function does and why it is needed?

[ahatanak]

Address review comments.

[ahatanak]

We could run another pass to clean up the IR after inlining, but I think it's better to do the cleanup right after the callee function is cloned in the caller function.

llvm/include/llvm/IR/Intrinsics.td

449 ↗

(On Diff #320001)

I'll change the other intrinsics in a separate patch.

[fhahn]

Thanks for the updates! The LLVM side of things now looks good to me. Could you also add a description of the new bundle to LangRef https://llvm.org/docs/LangRef.html#operand-bundles?

In D92808#2535593, @ahatanak wrote:

We could run another pass to clean up the IR after inlining, but I think it's better to do the cleanup right after the callee function is cloned in the caller function.

Thanks for elaborating. Given that it is isolated to a single place and the inliner already has logic to deal with a few different intrinsics/bundle types, I think that should be fine.

llvm/include/llvm/IR/Intrinsics.td

449 ↗

(On Diff #320001)

SGTM. thanks!

llvm/lib/Transforms/Utils/InlineFunction.cpp

1655

nit: typo autoreleasaeRV

1669

nit: Can you just use CB.getModule()?

1673

we are in the llvm:: namespace here I think, so this should not be needed ? above you use the objcarc namespace without llvm:: prefix as well. Same in other places in the function.

[ahatanak]

Address review comments.

[ahatanak]

Add a description of the new bundle to LangRef.

[fhahn]

Core LLVM parts LGTM, thanks! I'm going to mark this as accepted, given that the other parts have been reviewed earlier already

[ahatanak]

I ended up reverting the changes I made to llvm/lib/IR/AutoUpgrade.cpp as the file was including llvm/Analysis/ObjCARCUtil.h, which was violating layering.

[dexonsmith]

In D92808#2545873, @ahatanak wrote:

I ended up reverting the changes I made to llvm/lib/IR/AutoUpgrade.cpp as the file was including llvm/Analysis/ObjCARCUtil.h, which was violating layering.

Are you planning to land the upgrade another way?

[ahatanak]

I do have a plan to modify the auto upgrader to use the bundles when it's possible to do so, but I'm currently not considering landing the changes I reverted since they were only needed to avoid duplicating constant strings. I don't think we can avoid duplicating the string unless objcarc::getRVMarkerModuleFlagStr() can be moved to a file in llvm/include/llvm/IR.

[thakis]

This makes clang assert for some inputs. See https://bugs.chromium.org/p/chromium/issues/detail?id=1175886#c10 for a repro.

[thakis]

Here's a reduced repro:

thakis@thakis:~/src/llvm-project$ cat GREYAssertDefaultConfiguration-9e1f3b.reduced.m
inline id a() {}
void b() {
  a();
  a();
}
thakis@thakis:~/src/llvm-project$ out/gn/bin/clang -cc1 -cc1 -triple arm64-apple-ios12.2.0 -Oz -fobjc-arc -emit-llvm GREYAssertDefaultConfiguration-9e1f3b.reduced.m
GREYAssertDefaultConfiguration-9e1f3b.reduced.m:1:16: warning: non-void function does not return a value [-Wreturn-type]
inline id a() {}
               ^
clang: ../../llvm/include/llvm/IR/InstrTypes.h:1969: Optional<llvm::OperandBundleUse> llvm::CallBase::getOperandBundle(uint32_t) const: Assertion `countOperandBundlesOfType(ID) < 2 && "Precondition violated!"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: out/gn/bin/clang -cc1 -cc1 -triple arm64-apple-ios12.2.0 -Oz -fobjc-arc -emit-llvm GREYAssertDefaultConfiguration-9e1f3b.reduced.m
1.	<eof> parser at end of file
2.	Per-module optimization passes
3.	Running pass 'CallGraph Pass Manager' on module 'GREYAssertDefaultConfiguration-9e1f3b.reduced.m'.
 #0 0x00000000034a054c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Support/Unix/Signals.inc:565:13
 #1 0x000000000349e45e llvm::sys::RunSignalHandlers() /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Support/Signals.cpp:72:18
 #2 0x00000000034a089f SignalHandler(int) /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Support/Unix/Signals.inc:407:1
 #3 0x00007f8505e34140 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14140)
 #4 0x00007f850594cce1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x00007f8505936537 abort ./stdlib/abort.c:81:7
 #6 0x00007f850593640f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8
 #7 0x00007f850593640f _nl_load_domain ./intl/loadmsgcat.c:970:34
 #8 0x00007f8505945662 (/lib/x86_64-linux-gnu/libc.so.6+0x34662)
 #9 0x0000000002be88bd (out/gn/bin/clang+0x2be88bd)
#10 0x0000000003afdd35 hasValue /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/include/llvm/ADT/Optional.h:194:53
#11 0x0000000003afdd35 hasValue /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/include/llvm/ADT/Optional.h:286:52
#12 0x0000000003afdd35 hasRVOpBundle /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/include/llvm/Analysis/ObjCARCUtil.h:42:61
#13 0x0000000003afdd35 llvm::InlineFunction(llvm::CallBase&, llvm::InlineFunctionInfo&, llvm::AAResults*, bool, llvm::Function*) /usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Transforms/Utils/InlineFunction.cpp:1953:9

Given that the repro is so tiny, I've reverted this for now in de1966e5427985163f8e816834b3a0564b5e24cd.

[ahatanak]

It looks like there is a pre-existing bug in replaceUsesOfNonProtoConstant where the operand bundles of all call sites are accumulated into newBundles. This will be fixed if its declaration is moved into the loop body. Also, we found another case of deadargelim changing the return type of the called function to void. I'll post a new patch when I have the fixes.

[ahatanak]

This patch causes opt to crash when the following IR is compiled:

$ cat test.ll

@g0 = global i8 zeroinitializer, align 1

define internal i8* @foo() {
  ret i8* @g0
}

define void @test() {
  %r = call i8* @foo() [ "clang.arc.rv"(i64 1) ]
  call void (...) @llvm.objc.clang.arc.noop.use(i8* %r)
  ret void
}

declare void @llvm.objc.clang.arc.noop.use(...)

!llvm.module.flags = !{!0}

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

$ opt -ipsccp -deadargelim -objc-arc -o - -S test.ll

What's happening is that ipsccp replaces argument %r passed to call @llvm.objc.clang.arc.noop.use with @g0 and then deadargelim changes the return type of @foo to void since there are no explicit users of its return. ARC optimizer crashes because it expects a call with the operand bundle to have a return.

To fix the crash, we can modify tryToReplaceWithConstant to prevent ipsccp from replacing @llvm.objc.clang.arc.noop.use's argument. Alternatively, we can restore the check in DeadArgumentEliminationPass::SurveyFunction, which was in earlier versions of the patch (see https://reviews.llvm.org/D92808?id=319082#inline-892965). I feel like the latter option is better since we wouldn't' have to prevent ipsccp from optimizing the IR, which is perfectly legal. If it's not desirable to add a check that is too specific to ObjC, we could add a new generic function to the core library, which indicates the function's return type shouldn't be changed, and use it to tell deadargelim not to change the return type. Or we could use a bundle that is more generic than clang.arc.rv (for example, "implicitly.used.by"(@llvm.objc.retainAutoreleasedReturnValue), which indicates the result is used by an instruction that isn't explicitly emitted in the IR).

[dexonsmith]

I have a couple of suggestions below to add to the mix (but I don't feel strongly about either of them, just adding them for consideration).

In D92808#2552100, @ahatanak wrote:

define void @test() {
  %r = call i8* @foo() [ "clang.arc.rv"(i64 1) ]
  call void (...) @llvm.objc.clang.arc.noop.use(i8* %r)
  ret void
}

Firstly, it feels to me like this IR would clarify that the bundle looks at the return value:

define void @test() {
  %r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* %r) ]
  ret void
}

Notice this passes %r to the operand bundle, which the verifier rejects. I wonder if the IR rule could safely be weakened to allow an operand bundle to reference the call/invoke it's attached to? (Not sure what all the implications would be, or if this would be sufficient...)

In D92808#2552100, @ahatanak wrote:

Or we could use a bundle that is more generic than clang.arc.rv (for example, "implicitly.used.by"(@llvm.objc.retainAutoreleasedReturnValue), which indicates the result is used by an instruction that isn't explicitly emitted in the IR).

Secondly, I agree it might be cleaner to formalize a generic operand bundle that has the semantics ARC needs. It might be nice to have this also model the inlining semantics, maybe something like "returnfilter":

%x = call type @f1(...) [ "returnfilter"(type(type)* @f2) ]

The semantics could be that %x is implicitly passed through @f2. Concretely, maybe that's:

• %x has an implicit use.
• @f1's return type cannot be changed.
• Inlining @f1 generates explicit calls to @f2.
• The backend lowers the bundle as late as possible to calls to @f2.

Thirdly, you could combine the two previous ideas, generalizing the semantics to an "onreturn" bundle that is allowed to self-reference the callable, that the inliner knows about, etc., something like:

%x = call type @f1(...) [ "onreturn"(type(type)* @f2, type %x) ]

This @f2 is implicitly called and its return used in place of %x, but it explicitly lists its arguments. Compared to "returnfilter", it has the flexibility to specify argument order and might be more useful for other applications.

[rjmccall]

The ultimate code-generation consequences of this feature aren't very generic, so we shouldn't let ourselves get caught up designing an extremely general feature that ultimately either doesn't do what we need it to do or doesn't actually work for any of the generalizations.

[dexonsmith]

In D92808#2552354, @rjmccall wrote:

The ultimate code-generation consequences of this feature aren't very generic, so we shouldn't let ourselves get caught up designing an extremely general feature that ultimately either doesn't do what we need it to do or doesn't actually work for any of the generalizations.

Sure, those are two bad potential outcomes.

Stepping back, the goal of this patch is to encode ARC's code generation invariants in the IR so that maintainers don't inadvertently break them. I think the IR should describe those invariants as clearly and precisely as is reasonable.

Besides idle / misplaced optimism about reuse, I was trying to make the following points along those lines, so I'll restate them more directly:

• Explicitly adding a use of the call avoids having to teach transformations / analyses about it. It might be simpler to add a self-reference carve-out for this operand bundle (or operand bundles in general) than to teach all the passes that this operand bundle implies a use of the callsite (but I'm not sure; maybe there are lots of assumptions about self-references not existing outside of PHIs...).
• Passing the function that's implicitly called into the operand bundle is simpler to reason about than an opaque table index.
• Naming the operand bundle after its code generation consequences makes it simpler reason about. (A few generic names that make some sense to me (besides what I listed yesterday) include "callafter", "callimmediately", "forwardto", and "attachedcall". If it's important to make this ARC only, it could be (e.g.) "clang.arc.attachedcall".)

[ahatanak]

Fix two bugs which were discovered.

• Fix bug in replaceUsesOfNonProtoConstant.
• Teach SCCP not to replace the return of a "clang.arc.rv" call with a constant.

[ahatanak]

In D92808#2555737, @dexonsmith wrote:

In D92808#2552354, @rjmccall wrote:

The ultimate code-generation consequences of this feature aren't very generic, so we shouldn't let ourselves get caught up designing an extremely general feature that ultimately either doesn't do what we need it to do or doesn't actually work for any of the generalizations.

Sure, those are two bad potential outcomes.

Stepping back, the goal of this patch is to encode ARC's code generation invariants in the IR so that maintainers don't inadvertently break them. I think the IR should describe those invariants as clearly and precisely as is reasonable.

Besides idle / misplaced optimism about reuse, I was trying to make the following points along those lines, so I'll restate them more directly:

• Explicitly adding a use of the call avoids having to teach transformations / analyses about it. It might be simpler to add a self-reference carve-out for this operand bundle (or operand bundles in general) than to teach all the passes that this operand bundle implies a use of the callsite (but I'm not sure; maybe there are lots of assumptions about self-references not existing outside of PHIs...).
• Passing the function that's implicitly called into the operand bundle is simpler to reason about than an opaque table index.
• Naming the operand bundle after its code generation consequences makes it simpler reason about. (A few generic names that make some sense to me (besides what I listed yesterday) include "callafter", "callimmediately", "forwardto", and "attachedcall". If it's important to make this ARC only, it could be (e.g.) "clang.arc.attachedcall".)

The second and third suggestions seem like an improvement over the bundle used in this patch.

I'm not sure about the first one though as it seems to me that you'll still have to teach at least some of the passes about the self-reference.

For example, if SCCP just does a normal RAUW on the following call, which is taken from the example I posted,

%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* %r) ]

will become

%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* @g0) ]

%r doesn't have an explicit use in the IR anymore, so passes like deadargelim can change the return type of the function to void.

[fhahn]

In D92808#2555868, @ahatanak wrote:

In D92808#2555737, @dexonsmith wrote:

In D92808#2552354, @rjmccall wrote:

The ultimate code-generation consequences of this feature aren't very generic, so we shouldn't let ourselves get caught up designing an extremely general feature that ultimately either doesn't do what we need it to do or doesn't actually work for any of the generalizations.

Sure, those are two bad potential outcomes.

Stepping back, the goal of this patch is to encode ARC's code generation invariants in the IR so that maintainers don't inadvertently break them. I think the IR should describe those invariants as clearly and precisely as is reasonable.

Besides idle / misplaced optimism about reuse, I was trying to make the following points along those lines, so I'll restate them more directly:

• Explicitly adding a use of the call avoids having to teach transformations / analyses about it. It might be simpler to add a self-reference carve-out for this operand bundle (or operand bundles in general) than to teach all the passes that this operand bundle implies a use of the callsite (but I'm not sure; maybe there are lots of assumptions about self-references not existing outside of PHIs...).
• Passing the function that's implicitly called into the operand bundle is simpler to reason about than an opaque table index.
• Naming the operand bundle after its code generation consequences makes it simpler reason about. (A few generic names that make some sense to me (besides what I listed yesterday) include "callafter", "callimmediately", "forwardto", and "attachedcall". If it's important to make this ARC only, it could be (e.g.) "clang.arc.attachedcall".)

The second and third suggestions seem like an improvement over the bundle used in this patch.

+1, especially passing the function to call as argument to the bundle would make the lowering slightly easier; at the moment we may need to inject a new declaration during codegen.

I'm not sure about the first one though as it seems to me that you'll still have to teach at least some of the passes about the self-reference.

The patch already tries to do that, by adding calls to llvm.objc.clang.arc.noop.use, to make it explicit that there is a use of the returned value. This ensures most optimizations are aware of the fact that the return value is used. The problem Akira is that regular uses can be updated and are not enough to block the problematic transformation in SCCP.

If the use is directly at the bundle, I think we could somehow teach to lowering to use the referenced value and pass that to the ObjC runtime. But it would disable the runtime optimization.

I think in the short-term it would be OK to update SCCP as done in this patch, if we specify the arc bundle implicitly uses the return value, which does not refer to any ObjC specifics. We already do something similar for function with certain attributes.

I think it would be good to consider generalizing this 'implicitly used return value bundle' concept as a follow up, if there are other potential users. We then already laid the ground-work and updated the existing passes to work for the arc bundle and only have to update the checks.

@dexonsmith @rjmccall What do you think? Would you be happy with iterating on the suggestions in tree?

llvm/docs/LangRef.rst

2335 ↗

(On Diff #322867)

Can you add a sentence making it explicit that calls with that bundle implicitly use the returned value?

llvm/lib/Transforms/Scalar/SCCP.cpp

148 ↗

(On Diff #322867)

I think it would be slightly simpler to have a positive name for the set, e.g. MustPreserveReturnsInFn. What do you think? IMO that's more general than referring to 'not zapping'.

1643 ↗

(On Diff #322867)

I'd keep things generic and say something like Calls with "clang.arc.rv" implicitly use the return value and those uses cannot be updated with a constant..

[fhahn]

Another thing I noticed that there's verifier support missing. I think we should at least check that only a single clang.arc.rv bundle is specified (https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/Verifier.cpp#L3191). We should probably also enforce that the bundle is only provided for functions with an i8* return type. That can also be done after the main patch lands.

[dexonsmith]

In D92808#2555868, @ahatanak wrote:

For example, if SCCP just does a normal RAUW on the following call, which is taken from the example I posted,

%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* %r) ]

will become

%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* @g0) ]

%r doesn't have an explicit use in the IR anymore, so passes like deadargelim can change the return type of the function to void.

Is that a problem? It seems like this keeps all the information ARC needs. This could be lowered to:

call void @foo()
call @unsafeClaimAutoreleasedReturnValue(i8* @g0)

The ARC optimizer and ARC lowering both need to know about this case, but I don't see why deadargelim needs to know anything special.

[dexonsmith]

In D92808#2556542, @fhahn wrote:

@dexonsmith @rjmccall What do you think? Would you be happy with iterating on the suggestions in tree?

I don't feel strongly either way; happy for it to land as-is (bugs fixed) and iterated on there. One thought is that if the name is going to change it'd be nice to have it right for git-blame (today I would vote for "clang.arc.attachedcall" but anything a bit more clear would be fine), but of course that can change later too.

[ahatanak]

In D92808#2557634, @dexonsmith wrote:

In D92808#2555868, @ahatanak wrote:

For example, if SCCP just does a normal RAUW on the following call, which is taken from the example I posted,

%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* %r) ]

will become

%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* @g0) ]

%r doesn't have an explicit use in the IR anymore, so passes like deadargelim can change the return type of the function to void.

Is that a problem? It seems like this keeps all the information ARC needs. This could be lowered to:

call void @foo()
call @unsafeClaimAutoreleasedReturnValue(i8* @g0)

The ARC optimizer and ARC lowering both need to know about this case, but I don't see why deadargelim needs to know anything special.

I see, you are right.

[ahatanak]

Address all review comments except for the ones about the bundle name.

[ahatanak]

In D92808#2557132, @fhahn wrote:

Another thing I noticed that there's verifier support missing. I think we should at least check that only a single clang.arc.rv bundle is specified (https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/Verifier.cpp#L3191). We should probably also enforce that the bundle is only provided for functions with an i8* return type. That can also be done after the main patch lands.

I added the checks to the verifier. Note that the verifier accepts calls returning any pointer type since the return type isn't always i8* (e.g., NSObject *foo(void)).

[fhahn]

LGTM as initial step, but it would be good to adjust the name as Duncan suggested before landing.

[ahatanak]

• Renamed the operand bundle name to clang.arc.attachedcall. I also tried to pass the address of the ARC function as the argument, but the verifier doesn't allow taking addresses of intrinsics.

• Removed the call to @llvm.objc.clang.arc.noop.use, which in some cases was preventing the inliner to attach the operand bundle to a call in the callee function, when the operand bundle is removed from a call (see BundledRetainClaimRVs::eraseInst).

• Make sure emitOptimizedARCReturnCall doesn't drop any existing operand bundle on a call when adding the new operand bundle to the call.

[jyknight]

I ended up reverting the changes I made to llvm/lib/IR/AutoUpgrade.cpp as the file was including llvm/Analysis/ObjCARCUtil.h, which was violating layering.

It looks like it was not actually reverted in the version ultimately submitted. I've pushed commit 3c06676de14d7dd6dc3d1bf2989c503a2a5d438a which reverts the change to llvm/lib/IR/AutoUpgrade.cpp (duplicating the string constant).

[ahatanak]

Thanks! I didn't realize it hadn't been fixed in the patch I committed.

[thakis]

FYI, we're seeing test failures caused by this patch in iOS/arm64 builds when arc is used (simulator is fine though): https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c11

We'll try to investigate a bit more on our side, but I wanted to give an early(ish) heads-up in case others see this or whatnot.

Not sure if this landed before or after the 12.0 branch.

[ahatanak]

In D92808#2593204, @thakis wrote:

FYI, we're seeing test failures caused by this patch in iOS/arm64 builds when arc is used (simulator is fine though): https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c11

We'll try to investigate a bit more on our side, but I wanted to give an early(ish) heads-up in case others see this or whatnot.

Not sure if this landed before or after the 12.0 branch.

Thank you. Let me know when you have more information.

This isn't in release/12.x. The older version of the patch, which used attributes instead of operand bundles, was in release/12.x, but got reverted later.

[smeenai]

In D92808#2593331, @ahatanak wrote:

In D92808#2593204, @thakis wrote:

FYI, we're seeing test failures caused by this patch in iOS/arm64 builds when arc is used (simulator is fine though): https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c11

We'll try to investigate a bit more on our side, but I wanted to give an early(ish) heads-up in case others see this or whatnot.

Not sure if this landed before or after the 12.0 branch.

Thank you. Let me know when you have more information.

This isn't in release/12.x. The older version of the patch, which used attributes instead of operand bundles, was in release/12.x, but got reverted later.

Just to confirm, the revert is also in release/12.x, so I don't think there's any branch concerns here.

[thakis]

Thank you. Let me know when you have more information.

Repro is moving along. https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c26 and onward are getting pretty close.

[fhahn]

In D92808#2600031, @thakis wrote:

Thank you. Let me know when you have more information.

Repro is moving along. https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c26 and onward are getting pretty close.

That's interesting, thanks for sharing! I had a look at the linked good.s/bad.s and the sequence mentioned in the comments looks unexpected. mov x29, x29 should be exactly between a regular call and _objc_retainAutoreleasedReturnValue, but this is not what's happening in this case. I could not find a link to download the (pre-processed) source file. Would it be possible to get that file?

Ltmp239:
	bl	_objc_msgSend
Ltmp705:
	mov	x29, x29
Ltmp240:
	b	LBB19_3
LBB19_3:                                ; %invoke.cont4
	.loc	149 0 9                         ; ../../ios/chrome/browser/ui/infobars/infobar_container_coordinator.mm:0:9
	bl	_objc_retainAutoreleasedReturnValue

[hans]

The preprocessed source is attached to this comment: https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c26

good.s and bad.s are here: https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c28

And this shows the problematic difference between them: https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c30

After this change, for some reason x0 is getting clobbered in the prologue.

In https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c31 I verify that manually changing the register back to x1 fixes the issue.

[hans]

Reverted in https://github.com/llvm/llvm-project/commit/0a5dd067181dac2a8882a139ea3bd19bdea5fa44 until this can be fixed.

[fhahn]

In D92808#2600245, @hans wrote:

Reverted in https://github.com/llvm/llvm-project/commit/0a5dd067181dac2a8882a139ea3bd19bdea5fa44 until this can be fixed.

Thanks! I pushed a fix for the issue in 75805dce5ff8. @hans, would it be possible to check if D92808 + 75805dce5ff8 fixes the issue or should we just recommit D92808?

[hans]

In D92808#2601408, @fhahn wrote:

In D92808#2600245, @hans wrote:

Reverted in https://github.com/llvm/llvm-project/commit/0a5dd067181dac2a8882a139ea3bd19bdea5fa44 until this can be fixed.

Thanks! I pushed a fix for the issue in 75805dce5ff8. @hans, would it be possible to check if D92808 + 75805dce5ff8 fixes the issue or should we just recommit D92808?

Yes, I confirmed that 75805dce5ff8 fixes the test case I was looking at. Thanks!

[fhahn]

In D92808#2602687, @hans wrote:

In D92808#2601408, @fhahn wrote:

In D92808#2600245, @hans wrote:

Reverted in https://github.com/llvm/llvm-project/commit/0a5dd067181dac2a8882a139ea3bd19bdea5fa44 until this can be fixed.

Thanks! I pushed a fix for the issue in 75805dce5ff8. @hans, would it be possible to check if D92808 + 75805dce5ff8 fixes the issue or should we just recommit D92808?

Yes, I confirmed that 75805dce5ff8 fixes the test case I was looking at. Thanks!

Thanks for confirming!

[ahatanak]

Thanks! Reapplied the patch in https://github.com/llvm/llvm-project/commit/1900503595cbb84a4c6e140a9ba1a2c574c0586d

clang/lib/CodeGen/CGObjC.cpp

 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
 using namespace clang;
 using namespace CodeGen;
 
 typedef llvm::PointerIntPair<llvm::Value*,1,bool> TryEmitResult;
 static TryEmitResult
if (!mandatory && isa<llvm::Instruction>(result)) {
 
     call->setMetadata("clang.arc.copy_on_escape",
                       llvm::MDNode::get(Builder.getContext(), None));
   }
 
   return result;
 }
 
 static void emitAutoreleasedReturnValueMarker(CodeGenFunction &CGF) {
   // Fetch the void(void) inline asm which marks that we're going to
   // do something with the autoreleased return value.
   llvm::InlineAsm *&marker
     = CGF.CGM.getObjCEntrypoints().retainAutoreleasedReturnValueMarker;
   if (!marker) {
     StringRef assembly
       = CGF.CGM.getTargetCodeGenInfo()
            .getARCRetainAutoreleasedReturnValueMarker();
 
     // If we have an empty assembly string, there's nothing to do.
     if (assembly.empty()) {
 
     // Otherwise, at -O0, build an inline asm that we're going to call
     // in a moment.
     } else if (CGF.CGM.getCodeGenOpts().OptimizationLevel == 0) {
       llvm::FunctionType *type =
         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 {
-      const char *markerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
-      if (!CGF.CGM.getModule().getModuleFlag(markerKey)) {
+      const char *retainRVMarkerKey = llvm::objcarc::getRVMarkerModuleFlagStr();
+      if (!CGF.CGM.getModule().getModuleFlag(retainRVMarkerKey)) {
         auto *str = llvm::MDString::get(CGF.getLLVMContext(), assembly);
-        CGF.CGM.getModule().addModuleFlag(llvm::Module::Error, markerKey, str);
+        CGF.CGM.getModule().addModuleFlag(llvm::Module::Error,
+                                          retainRVMarkerKey, 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));
 }
 
+static llvm::Value *emitOptimizedARCReturnCall(llvm::Value *value,
+                                               bool IsRetainRV,
+                                               CodeGenFunction &CGF) {
+  emitAutoreleasedReturnValueMarker(CGF);
+
+  // Annotate the call with attributes instead of emitting retainRV or claimRV
+  // calls in the IR. We currently do this only when the optimization level
+  // isn't -O0 since global-isel, which is currently run at -O0, doesn't know
+  // about the attributes.
+
+  // FIXME: Do this when the target isn't aarch64.
+  if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+      CGF.CGM.getTarget().getTriple().isAArch64()) {
+    auto *callBase = cast<llvm::CallBase>(value);
+    llvm::AttributeList attrs = callBase->getAttributes();
+    attrs = attrs.addAttribute(CGF.getLLVMContext(),
+                               llvm::AttributeList::ReturnIndex,
+                               llvm::objcarc::getRVAttrKeyStr(),
+                               llvm::objcarc::getRVAttrValStr(IsRetainRV));
+    callBase->setAttributes(attrs);
+    return callBase;
+  }
+
+  bool isNoTail =
+      CGF.CGM.getTargetCodeGenInfo().markARCOptimizedReturnCallsAsNoTail();
+  llvm::CallInst::TailCallKind tailKind =
+      isNoTail ? llvm::CallInst::TCK_NoTail : llvm::CallInst::TCK_None;
+  ObjCEntrypoints &EPs = CGF.CGM.getObjCEntrypoints();
+  llvm::Function *&EP = IsRetainRV
+                            ? EPs.objc_retainAutoreleasedReturnValue
+                            : EPs.objc_unsafeClaimAutoreleasedReturnValue;
+  llvm::Intrinsic::ID IID =
+      IsRetainRV ? llvm::Intrinsic::objc_retainAutoreleasedReturnValue
+                 : llvm::Intrinsic::objc_unsafeClaimAutoreleasedReturnValue;
+  return emitARCValueOperation(CGF, value, nullptr, EP, IID, tailKind);
+}
+
 /// 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) {
-  emitAutoreleasedReturnValueMarker(*this);
-  llvm::CallInst::TailCallKind tailKind =
-      CGM.getTargetCodeGenInfo().markARCOptimizedReturnCallsAsNoTail()
-          ? llvm::CallInst::TCK_NoTail
-          : llvm::CallInst::TCK_None;
-  return emitARCValueOperation(
-      *this, value, nullptr,
-      CGM.getObjCEntrypoints().objc_retainAutoreleasedReturnValue,
-      llvm::Intrinsic::objc_retainAutoreleasedReturnValue, tailKind);
+  return emitOptimizedARCReturnCall(value, true, *this);
 }
 
 /// 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
 /// __unsafe_unretained variable.
 ///
 ///   call i8* \@objc_unsafeClaimAutoreleasedReturnValue(i8* %value)
 llvm::Value *
 CodeGenFunction::EmitARCUnsafeClaimAutoreleasedReturnValue(llvm::Value *value) {
-  emitAutoreleasedReturnValueMarker(*this);
-  llvm::CallInst::TailCallKind tailKind =
-      CGM.getTargetCodeGenInfo().markARCOptimizedReturnCallsAsNoTail()
-          ? llvm::CallInst::TCK_NoTail
-          : llvm::CallInst::TCK_None;
-  return emitARCValueOperation(
-      *this, value, nullptr,
-      CGM.getObjCEntrypoints().objc_unsafeClaimAutoreleasedReturnValue,
-      llvm::Intrinsic::objc_unsafeClaimAutoreleasedReturnValue, tailKind);
+  return emitOptimizedARCReturnCall(value, false, *this);
 }
 
 /// Release the given object.
 ///   call void \@objc_release(i8* %value)
 void CodeGenFunction::EmitARCRelease(llvm::Value *value,
                                      ARCPreciseLifetime_t precise) {
   if (isa<llvm::ConstantPointerNull>(value)) return;
 

clang/test/CodeGenObjC/arc-rv-attr.m

+// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK
+
+@class A;
+
+A *makeA(void);
+
+void test_assign() {
+  __unsafe_unretained id x;
+  x = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_assign()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="claim" [[A:.*]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_assign_assign() {
+  __unsafe_unretained id x, y;
+  x = y = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_assign_assign()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[Y:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="claim" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_strong_assign_assign() {
+  __strong id x;
+  __unsafe_unretained id y;
+  x = y = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_strong_assign_assign()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[Y:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="retain" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT:    [[OLD:%.*]] = load i8*, i8** [[X]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[OLD]]
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    [[T0:%.*]] = load i8*, i8** [[X]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[T0]])
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_assign_strong_assign() {
+  __unsafe_unretained id x;
+  __strong id y;
+  x = y = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_assign_strong_assign()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[Y:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="retain" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    [[OLD:%.*]] = load i8*, i8** [[Y]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[OLD]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    [[T0:%.*]] = load i8*, i8** [[Y]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[T0]])
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_init() {
+  __unsafe_unretained id x = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_init()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="claim" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_init_assignment() {
+  __unsafe_unretained id x;
+  __unsafe_unretained id y = x = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_init_assignment()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[Y:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="claim" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_strong_init_assignment() {
+  __unsafe_unretained id x;
+  __strong id y = x = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_strong_init_assignment()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[Y:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="retain" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT:    [[T0:%.*]] = load i8*, i8** [[Y]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[T0]])
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_init_strong_assignment() {
+  __strong id x;
+  __unsafe_unretained id y = x = makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_init_strong_assignment()
+// CHECK:         [[X:%.*]] = alloca i8*
+// CHECK:         [[Y:%.*]] = alloca i8*
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="retain" [[A]]* @makeA()
+// CHECK-NEXT:    [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
+// CHECK-NEXT:    [[OLD:%.*]] = load i8*, i8** [[X]]
+// CHECK-NEXT:    store i8* [[T1]], i8** [[X]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[OLD]])
+// CHECK-NEXT:    store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    [[T0:%.*]] = load i8*, i8** [[X]]
+// CHECK-NEXT:    call void @llvm.objc.release(i8* [[T0]])
+// CHECK-NEXT:    bitcast
+// CHECK-NEXT:    lifetime.end
+// CHECK-NEXT:    ret void
+
+void test_ignored() {
+  makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_ignored()
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="claim" [[A]]* @makeA()
+// CHECK-NEXT:    ret void
+
+void test_cast_to_void() {
+  (void) makeA();
+}
+// CHECK-LABEL: define{{.*}} void @test_cast_to_void()
+// CHECK:         [[T0:%.*]] = call "clang.arc.rv"="claim" [[A]]* @makeA()
+// CHECK-NEXT:    ret void
+
+// This is always at the end of the module.
+
+// CHECK-OPTIMIZED: !llvm.module.flags = !{!0,
+// CHECK-OPTIMIZED: !0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov{{.*}}marker for objc_retainAutoreleaseReturnValue"}

clang/test/CodeGenObjC/arc-unsafeclaim.m

 //   Make sure it works on x86-64.
 // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime=macosx-10.11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=NOTAIL-CALL
 
 //   Make sure it works on x86-32.
 // RUN: %clang_cc1 -triple i386-apple-darwin11 -fobjc-runtime=macosx-fragile-10.11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED -check-prefix=CALL
 
-//   Make sure it works on ARM.
+//   Make sure it works on ARM64.
 // RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED -check-prefix=CALL
-// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPTIMIZED -check-prefix=CALL
 
-//   Make sure it works on ARM64.
+//   Make sure it works on ARM.
 // RUN: %clang_cc1 -triple armv7-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED -check-prefix=CALL
 // RUN: %clang_cc1 -triple armv7-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPTIMIZED -check-prefix=CALL
 
 //   Make sure that it's implicitly disabled if the runtime version isn't high enough.
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-10.10 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=DISABLED
 // RUN: %clang_cc1 -triple arm64-apple-ios8 -fobjc-runtime=ios-8 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=DISABLED -check-prefix=DISABLED-MARKED
 
 @class A;

llvm/include/llvm/Analysis/ObjCARCRVAttr.h

+//===- ObjCARCRVAttr.h - ObjC ARC Attribute Analysis ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file defines functions which look for or remove attributes retainRV,
+/// claimRV, and rv_marker.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_ANALYSIS_OBJCARCRVATTR_H
+#define LLVM_LIB_ANALYSIS_OBJCARCRVATTR_H
+
+#include "llvm/IR/InstrTypes.h"
+
+namespace llvm {
+namespace objcarc {
+
+static inline const char *getRVMarkerModuleFlagStr() {
+  return "clang.arc.retainAutoreleasedReturnValueMarker";
+}
+
+static inline const char *getRVAttrKeyStr() { return "clang.arc.rv"; }
+
+static inline const char *getRVAttrValStr(bool Retain) {
+  return Retain ? "retain" : "claim";
+}
+
+static inline bool hasRetainRVAttr(const CallBase *CB) {
+  return CB->getAttribute(llvm::AttributeList::ReturnIndex, getRVAttrKeyStr())
+      .getValueAsString()
+      .equals(getRVAttrValStr(true));
+}
+static inline bool hasClaimRVAttr(const CallBase *CB) {
+  return CB->getAttribute(llvm::AttributeList::ReturnIndex, getRVAttrKeyStr())
+      .getValueAsString()
+      .equals(getRVAttrValStr(false));
+}
+
+static inline bool hasRetainRVOrClaimRVAttr(const CallBase *CB) {
+  return CB->hasRetAttr(getRVAttrKeyStr());
+}
+
+static inline void removeRetainRVOrClaimRVAttr(CallBase *CB) {
+  CB->removeAttribute(llvm::AttributeList::ReturnIndex, getRVAttrKeyStr());
+}
+
+} // end namespace objcarc
+} // end namespace llvm
+
+#endif

llvm/lib/IR/AutoUpgrade.cpp

 //===-- AutoUpgrade.cpp - Implement auto-upgrade helper functions ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the auto-upgrade helper functions.
 // This is where deprecated IR intrinsics and other IR features are updated to
 // current specifications.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
-#include "llvm/IR/Instruction.h"
 #include "llvm/IR/InstVisitor.h"
+#include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsAArch64.h"
 #include "llvm/IR/IntrinsicsARM.h"
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
bool llvm::UpgradeDebugInfo(Module &M) {
   }
   return Modified;
 }
 
 /// This checks for objc retain release marker which should be upgraded. It
 /// returns true if module is modified.
 static bool UpgradeRetainReleaseMarker(Module &M) {
   bool Changed = false;
-  const char *MarkerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
+  const char *MarkerKey = objcarc::getRVMarkerModuleFlagStr();
   NamedMDNode *ModRetainReleaseMarker = M.getNamedMetadata(MarkerKey);
   if (ModRetainReleaseMarker) {
     MDNode *Op = ModRetainReleaseMarker->getOperand(0);
     if (Op) {
       MDString *ID = dyn_cast_or_null<MDString>(Op->getOperand(0));
       if (ID) {
         SmallVector<StringRef, 4> ValueComp;
         ID->getString().split(ValueComp, "#");

llvm/lib/IR/Instruction.cpp

 //===-- Instruction.cpp - Implement the Instruction class -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Instruction class for the IR library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/Instruction.h"
-#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 using namespace llvm;
 
 Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
                          Instruction *InsertBefore)
   : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(nullptr) {
bool Instruction::mayWriteToMemory() const {
   case Instruction::VAArg:
   case Instruction::AtomicCmpXchg:
   case Instruction::AtomicRMW:
   case Instruction::CatchPad:
   case Instruction::CatchRet:
     return true;
   case Instruction::Call:
   case Instruction::Invoke:
-  case Instruction::CallBr:
-    return !cast<CallBase>(this)->onlyReadsMemory();
+  case Instruction::CallBr: {
+    if (!cast<CallBase>(this)->onlyReadsMemory())
+      return true;
+    if (auto *CB = dyn_cast<CallBase>(this))
+      return objcarc::hasRetainRVOrClaimRVAttr(CB);
+    return false;
+  }
   case Instruction::Load:
     return !cast<LoadInst>(this)->isUnordered();
   }
 }
 
 bool Instruction::isAtomic() const {
   switch (getOpcode()) {
   default:

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/CodeGen/CallingConvLower.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
   if (IsTailCall) {
     MF.getFrameInfo().setHasTailCall();
     SDValue Ret = DAG.getNode(AArch64ISD::TC_RETURN, DL, NodeTys, Ops);
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
     return Ret;
   }
 
   unsigned CallOpc = AArch64ISD::CALL;
-  // Calls marked with "rv_marker" are special. They should be expanded to the
-  // call, directly followed by a special marker sequence. Use the CALL_RVMARKER
-  // to do that.
-  if (CLI.CB && CLI.CB->hasRetAttr("rv_marker")) {
-    assert(!IsTailCall && "tail calls cannot be marked with rv_marker");
+  // Calls marked with "clang.arc.rv" are special. They should be expanded to
+  // the call, directly followed by a special marker sequence. Use the
+  // CALL_RVMARKER to do that.
+  if (CLI.CB && CLI.CB->hasRetAttr(objcarc::getRVAttrKeyStr())) {
+    assert(!IsTailCall && "tail calls cannot be marked with clang.arc.rv");
     CallOpc = AArch64ISD::CALL_RVMARKER;
   }
 
   // Returns a chain and a flag for retval copy to use.
   Chain = DAG.getNode(CallOpc, DL, NodeTys, Ops);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   InFlag = Chain.getValue(1);
   DAG.addCallSiteInfo(Chain.getNode(), std::move(CSInfo));

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

 // This pass is often useful as a cleanup pass to run after aggressive
 // interprocedural passes, which add possibly-dead arguments or return values.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/IPO/DeadArgumentElimination.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstrTypes.h"
for (const Use &U : F.uses()) {
 
     // If we end up here, we are looking at a direct call to our function.
 
     // Now, check how our return value(s) is/are used in this caller. Don't
     // bother checking return values if all of them are live already.
     if (NumLiveRetVals == RetCount)
       continue;
 
+    // Don't change the return type of the function if it has retainRV/claimRV.
+    if (objcarc::hasRetainRVOrClaimRVAttr(CB)) {
+      NumLiveRetVals = RetCount;
+      RetValLiveness.assign(RetCount, Live);
+      continue;
+    }
+
     // Check all uses of the return value.
     for (const Use &U : CB->uses()) {
       if (ExtractValueInst *Ext = dyn_cast<ExtractValueInst>(U.getUser())) {
         // This use uses a part of our return value, survey the uses of
         // that part and store the results for this index only.
         unsigned Idx = *Ext->idx_begin();
         if (RetValLiveness[Idx] != Live) {
           RetValLiveness[Idx] = SurveyUses(Ext, MaybeLiveRetUses[Idx]);

llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h

 enum class ARCRuntimeEntryPointKind {
   AutoreleaseRV,
   Release,
   Retain,
   RetainBlock,
   Autorelease,
   StoreStrong,
   RetainRV,
+  ClaimRV,
   RetainAutorelease,
   RetainAutoreleaseRV,
 };
 
 /// Declarations for ObjC runtime functions and constants. These are initialized
 /// lazily to avoid cluttering up the Module with unused declarations.
 class ARCRuntimeEntryPoints {
 public:
   ARCRuntimeEntryPoints() = default;
 
   void init(Module *M) {
     TheModule = M;
     AutoreleaseRV = nullptr;
     Release = nullptr;
     Retain = nullptr;
     RetainBlock = nullptr;
     Autorelease = nullptr;
     StoreStrong = nullptr;
     RetainRV = nullptr;
+    ClaimRV = nullptr;
     RetainAutorelease = nullptr;
     RetainAutoreleaseRV = nullptr;
   }
 
   Function *get(ARCRuntimeEntryPointKind kind) {
     assert(TheModule != nullptr && "Not initialized.");
 
     switch (kind) {
     case ARCRuntimeEntryPointKind::AutoreleaseRV:
       return getIntrinsicEntryPoint(AutoreleaseRV,
                                     Intrinsic::objc_autoreleaseReturnValue);
     case ARCRuntimeEntryPointKind::Release:
       return getIntrinsicEntryPoint(Release, Intrinsic::objc_release);
     case ARCRuntimeEntryPointKind::Retain:
       return getIntrinsicEntryPoint(Retain, Intrinsic::objc_retain);
     case ARCRuntimeEntryPointKind::RetainBlock:
       return getIntrinsicEntryPoint(RetainBlock, Intrinsic::objc_retainBlock);
     case ARCRuntimeEntryPointKind::Autorelease:
       return getIntrinsicEntryPoint(Autorelease, Intrinsic::objc_autorelease);
     case ARCRuntimeEntryPointKind::StoreStrong:
       return getIntrinsicEntryPoint(StoreStrong, Intrinsic::objc_storeStrong);
     case ARCRuntimeEntryPointKind::RetainRV:
       return getIntrinsicEntryPoint(RetainRV,
                                 Intrinsic::objc_retainAutoreleasedReturnValue);
+    case ARCRuntimeEntryPointKind::ClaimRV:
+      return getIntrinsicEntryPoint(
+          ClaimRV, Intrinsic::objc_unsafeClaimAutoreleasedReturnValue);
     case ARCRuntimeEntryPointKind::RetainAutorelease:
       return getIntrinsicEntryPoint(RetainAutorelease,
                                     Intrinsic::objc_retainAutorelease);
     case ARCRuntimeEntryPointKind::RetainAutoreleaseRV:
       return getIntrinsicEntryPoint(RetainAutoreleaseRV,
                                 Intrinsic::objc_retainAutoreleaseReturnValue);
     }
 
private:
   Function *Autorelease = nullptr;
 
   /// Declaration for objc_storeStrong().
   Function *StoreStrong = nullptr;
 
   /// Declaration for objc_retainAutoreleasedReturnValue().
   Function *RetainRV = nullptr;
 
+  /// Declaration for objc_unsafeClaimAutoreleasedReturnValue().
+  Function *ClaimRV = nullptr;
+
   /// Declaration for objc_retainAutorelease().
   Function *RetainAutorelease = nullptr;
 
   /// Declaration for objc_retainAutoreleaseReturnValue().
   Function *RetainAutoreleaseRV = nullptr;
 
   Function *getIntrinsicEntryPoint(Function *&Decl, Intrinsic::ID IntID) {
     if (Decl)

llvm/lib/Transforms/ObjCARC/ObjCARC.h

 /// used. Naive LLVM IR transformations which would otherwise be
 /// behavior-preserving may break these assumptions.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TRANSFORMS_OBJCARC_OBJCARC_H
 #define LLVM_LIB_TRANSFORMS_OBJCARC_OBJCARC_H
 
+#include "ARCRuntimeEntryPoints.h"
+#include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/Analysis/ObjCARCAnalysisUtils.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/Transforms/Utils/Local.h"
 
 namespace llvm {
 namespace objcarc {
 
 /// Erase the given instruction.
 ///
 /// Many ObjC calls return their argument verbatim,
for (; I < E; ++I) {
       if (PNOpnd != POpnd)
         break;
     }
     if (I == E)
       PHIList.push_back(&P);
   }
 }
 
+static inline MDString *getRVInstMarker(Module &M) {
+  const char *MarkerKey = getRVMarkerModuleFlagStr();
+  return dyn_cast_or_null<MDString>(M.getModuleFlag(MarkerKey));
+}
+
+/// Create a call instruction with the correct funclet token. This should be
+/// called instead of calling CallInst::Create directly unless the call is
+/// going to be removed from the IR before WinEHPrepare.
+CallInst *createCallInstWithColors(
+    FunctionCallee Func, ArrayRef<Value *> Args, const Twine &NameStr,
+    Instruction *InsertBefore,
+    const DenseMap<BasicBlock *, ColorVector> &BlockColors);
+
+class BundledRetainClaimRVs {
+public:
+  BundledRetainClaimRVs(ARCRuntimeEntryPoints &P, bool ContractPass)
+      : EP(P), ContractPass(ContractPass) {}
+  ~BundledRetainClaimRVs();
+
+  /// Insert a retainRV/claimRV call to the normal destination blocks of invokes
+  /// annotated with retainRV/claimRV. If the edge to the normal destination
+  /// block is a critical edge, split it.
+  std::pair<bool, bool> insertAfterInvokes(Function &F, DominatorTree *DT);
+
+  /// Insert a retainRV/claimRV call.
+  CallInst *insertRVCall(Instruction *InsertPt, CallBase *AnnotatedCall);
+
+  /// Insert a retainRV/claimRV call with colors.
+  CallInst *insertRVCallWithColors(
+      Instruction *InsertPt, CallBase *AnnotatedCall,
+      const DenseMap<BasicBlock *, ColorVector> &BlockColors);
+
+  /// See if an instruction is a bundled retainRV/claimRV call.
+  bool contains(const Instruction *I) const {
+    if (auto *CI = dyn_cast<CallInst>(I))
+      return RVCalls.count(CI);
+    return false;
+  }
+
+  /// Remove a retainRV/claimRV call entirely.
+  void eraseInst(CallInst *CI) {
+    auto It = RVCalls.find(CI);
+    if (It != RVCalls.end()) {
+      objcarc::removeRetainRVOrClaimRVAttr(It->second);
+      RVCalls.erase(It);
+    }
+    EraseInstruction(CI);
+  }
+
+private:
+  /// A map of inserted retainRV/claimRV calls to annotated calls/invokes.
+  DenseMap<CallInst *, CallBase *> RVCalls;
+
+  ARCRuntimeEntryPoints &EP;
+  bool ContractPass;
+};
+
 } // end namespace objcarc
 } // end namespace llvm
 
 #endif

llvm/lib/Transforms/ObjCARC/ObjCARC.cpp

 //===-- ObjCARC.cpp -------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements common infrastructure for libLLVMObjCARCOpts.a, which
 // implements several scalar transformations over the LLVM intermediate
 // representation, including the C bindings for that library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ObjCARC.h"
 #include "llvm-c/Initialization.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/InitializePasses.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
 
 namespace llvm {
   class PassRegistry;
 }
 
 using namespace llvm;
 using namespace llvm::objcarc;
 
 /// initializeObjCARCOptsPasses - Initialize all passes linked into the
 /// ObjCARCOpts library.
 void llvm::initializeObjCARCOpts(PassRegistry &Registry) {
   initializeObjCARCAAWrapperPassPass(Registry);
   initializeObjCARCAPElimPass(Registry);
   initializeObjCARCExpandPass(Registry);
   initializeObjCARCContractLegacyPassPass(Registry);
   initializeObjCARCOptLegacyPassPass(Registry);
   initializePAEvalPass(Registry);
 }
 
 void LLVMInitializeObjCARCOpts(LLVMPassRegistryRef R) {
   initializeObjCARCOpts(*unwrap(R));
 }
+
+CallInst *llvm::objcarc::createCallInstWithColors(
+    FunctionCallee Func, ArrayRef<Value *> Args, const Twine &NameStr,
+    Instruction *InsertBefore,
+    const DenseMap<BasicBlock *, ColorVector> &BlockColors) {
+  FunctionType *FTy = Func.getFunctionType();
+  Value *Callee = Func.getCallee();
+  SmallVector<OperandBundleDef, 1> OpBundles;
+
+  if (!BlockColors.empty()) {
+    const ColorVector &CV = BlockColors.find(InsertBefore->getParent())->second;
+    assert(CV.size() == 1 && "non-unique color for block!");
+    Instruction *EHPad = CV.front()->getFirstNonPHI();
+    if (EHPad->isEHPad())
+      OpBundles.emplace_back("funclet", EHPad);
+  }
+
+  return CallInst::Create(FTy, Callee, Args, OpBundles, NameStr, InsertBefore);
+}
+
+std::pair<bool, bool>
+BundledRetainClaimRVs::insertAfterInvokes(Function &F, DominatorTree *DT) {
+  bool Changed = false, CFGChanged = false;
+
+  for (BasicBlock &BB : F) {
+    auto *I = dyn_cast<InvokeInst>(BB.getTerminator());
+
+    if (!I)
+      continue;
+
+    if (!objcarc::hasRetainRVOrClaimRVAttr(I))
+      continue;
+
+    BasicBlock *DestBB = I->getNormalDest();
+
+    if (!DestBB->getSinglePredecessor()) {
+      assert(I->getSuccessor(0) == DestBB &&
+             "the normal dest is expected to be the first successor");
+      DestBB = llvm::SplitCriticalEdge(I, 0, CriticalEdgeSplittingOptions(DT));
+      CFGChanged = true;
+    }
+
+    // We don't have to call insertRVCallWithColors since DestBB is the normal
+    // destination of the invoke.
+    insertRVCall(&*DestBB->getFirstInsertionPt(), I);
+    Changed = true;
+  }
+
+  return std::make_pair(Changed, CFGChanged);
+}
+
+CallInst *BundledRetainClaimRVs::insertRVCall(Instruction *InsertPt,
+                                              CallBase *AnnotatedCall) {
+  DenseMap<BasicBlock *, ColorVector> BlockColors;
+  return insertRVCallWithColors(InsertPt, AnnotatedCall, BlockColors);
+}
+
+CallInst *BundledRetainClaimRVs::insertRVCallWithColors(
+    Instruction *InsertPt, CallBase *AnnotatedCall,
+    const DenseMap<BasicBlock *, ColorVector> &BlockColors) {
+  IRBuilder<> Builder(InsertPt);
+  bool IsRetainRV = objcarc::hasRetainRVAttr(AnnotatedCall);
+  Function *Func = EP.get(IsRetainRV ? ARCRuntimeEntryPointKind::RetainRV
+                                     : ARCRuntimeEntryPointKind::ClaimRV);
+  Type *ParamTy = Func->getArg(0)->getType();
+  Value *CallArg = Builder.CreateBitCast(AnnotatedCall, ParamTy);
+  auto *Call =
+      createCallInstWithColors(Func, CallArg, "", InsertPt, BlockColors);
+  RVCalls[Call] = AnnotatedCall;
+  return Call;
+}
+
+BundledRetainClaimRVs::~BundledRetainClaimRVs() {
+  if (ContractPass) {
+    // At this point, we know that the annotated calls can't be tail calls as
+    // they are followed by marker instructions and retainRV/claimRV calls. Mark
+    // them as notail, so that the backend knows these calls can't be tail
+    // calls.
+    for (auto P : RVCalls)
+      if (auto *CI = dyn_cast<CallInst>(P.second))
+        CI->setTailCallKind(CallInst::TCK_NoTail);
+  } else {
+    for (auto P : RVCalls)
+      EraseInstruction(P.first);
+  }
+
+  RVCalls.clear();
+}

llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp

 
 #include "ARCRuntimeEntryPoints.h"
 #include "DependencyAnalysis.h"
 #include "ObjCARC.h"
 #include "ProvenanceAnalysis.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/EHPersonalities.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"

 namespace {
 /// Late ARC optimizations
 ///
 /// These change the IR in a way that makes it difficult to be analyzed by
 /// ObjCARCOpt, so it's run late.
 
 class ObjCARCContract {
   bool Changed;
+  bool CFGChanged;
   AAResults *AA;
   DominatorTree *DT;
   ProvenanceAnalysis PA;
   ARCRuntimeEntryPoints EP;
-
-  /// A flag indicating whether this optimization pass should run.
-  bool Run;
+  BundledRetainClaimRVs *BundledInsts = nullptr;
 
   /// The inline asm string to insert between calls and RetainRV calls to make
   /// the optimization work on targets which need it.
   const MDString *RVInstMarker;
 
   /// The set of inserted objc_storeStrong calls. If at the end of walking the
   /// function we have found no alloca instructions, these calls can be marked
   /// "tail".
class ObjCARCContract {
 
   void tryToContractReleaseIntoStoreStrong(
       Instruction *Release, inst_iterator &Iter,
       const DenseMap<BasicBlock *, ColorVector> &BlockColors);
 
 public:
   bool init(Module &M);
   bool run(Function &F, AAResults *AA, DominatorTree *DT);
+  bool hasCFGChanged() const { return CFGChanged; }
 };
 
 class ObjCARCContractLegacyPass : public FunctionPass {
   ObjCARCContract OCARCC;
 
 public:
   void getAnalysisUsage(AnalysisUsage &AU) const override;
   bool doInitialization(Module &M) override;
findRetainForStoreStrongContraction(Value *New, StoreInst *Store,
   Instruction *Retain = &*I;
   if (GetBasicARCInstKind(Retain) != ARCInstKind::Retain)
     return nullptr;
   if (GetArgRCIdentityRoot(Retain) != New)
     return nullptr;
   return Retain;
 }
 
-/// Create a call instruction with the correct funclet token. Should be used
-/// instead of calling CallInst::Create directly.
-static CallInst *
-createCallInst(FunctionType *FTy, Value *Func, ArrayRef<Value *> Args,
-               const Twine &NameStr, Instruction *InsertBefore,
-               const DenseMap<BasicBlock *, ColorVector> &BlockColors) {
-  SmallVector<OperandBundleDef, 1> OpBundles;
-  if (!BlockColors.empty()) {
-    const ColorVector &CV = BlockColors.find(InsertBefore->getParent())->second;
-    assert(CV.size() == 1 && "non-unique color for block!");
-    Instruction *EHPad = CV.front()->getFirstNonPHI();
-    if (EHPad->isEHPad())
-      OpBundles.emplace_back("funclet", EHPad);
-  }
-
-  return CallInst::Create(FTy, Func, Args, OpBundles, NameStr, InsertBefore);
-}
-
-static CallInst *
-createCallInst(FunctionCallee Func, ArrayRef<Value *> Args, const Twine &NameStr,
-               Instruction *InsertBefore,
-               const DenseMap<BasicBlock *, ColorVector> &BlockColors) {
-  return createCallInst(Func.getFunctionType(), Func.getCallee(), Args, NameStr,
-                        InsertBefore, BlockColors);
-}
-
 /// Attempt to merge an objc_release with a store, load, and objc_retain to form
 /// an objc_storeStrong. An objc_storeStrong:
 ///
 ///   objc_storeStrong(i8** %old_ptr, i8* new_value)
 ///
 /// is equivalent to the following IR sequence:
 ///
 ///   ; Load old value.
void ObjCARCContract::tryToContractReleaseIntoStoreStrong(
   Type *I8XX = PointerType::getUnqual(I8X);
 
   Value *Args[] = { Load->getPointerOperand(), New };
   if (Args[0]->getType() != I8XX)
     Args[0] = new BitCastInst(Args[0], I8XX, "", Store);
   if (Args[1]->getType() != I8X)
     Args[1] = new BitCastInst(Args[1], I8X, "", Store);
   Function *Decl = EP.get(ARCRuntimeEntryPointKind::StoreStrong);
-  CallInst *StoreStrong = createCallInst(Decl, Args, "", Store, BlockColors);
+  CallInst *StoreStrong =
+      objcarc::createCallInstWithColors(Decl, Args, "", Store, BlockColors);
   StoreStrong->setDoesNotThrow();
   StoreStrong->setDebugLoc(Store->getDebugLoc());
 
   // We can't set the tail flag yet, because we haven't yet determined
   // whether there are any escaping allocas. Remember this call, so that
   // we can set the tail flag once we know it's safe.
   StoreStrongCalls.insert(StoreStrong);
 
case ARCInstKind::Retain:
     // calls.
     if (!optimizeRetainCall(F, Inst))
       return false;
     // If we succeed in our optimization, fall through.
     LLVM_FALLTHROUGH;
   case ARCInstKind::RetainRV:
   case ARCInstKind::ClaimRV: {
     // If we're compiling for a target which needs a special inline-asm
-    // marker to do the return value optimization, insert it now.
+    // marker to do the return value optimization and the retainRV/claimRV call
+    // wasn't bundled with a call, insert the marker now.
     if (!RVInstMarker)
       return false;
+
+    if (BundledInsts->contains(Inst))
+      return false;
+
     BasicBlock::iterator BBI = Inst->getIterator();
     BasicBlock *InstParent = Inst->getParent();
 
     // Step up to see if the call immediately precedes the RV call.
     // If it's an invoke, we have to cross a block boundary. And we have
     // to carefully dodge no-op instructions.
     do {
       if (BBI == InstParent->begin()) {
         BasicBlock *Pred = InstParent->getSinglePredecessor();
if (GetRCIdentityRoot(&*BBI) == GetArgRCIdentityRoot(Inst)) {
                            "optimization.\n");
       Changed = true;
       InlineAsm *IA =
           InlineAsm::get(FunctionType::get(Type::getVoidTy(Inst->getContext()),
                                            /*isVarArg=*/false),
                          RVInstMarker->getString(),
                          /*Constraints=*/"", /*hasSideEffects=*/true);
 
-      createCallInst(IA, None, "", Inst, BlockColors);
+      objcarc::createCallInstWithColors(IA, None, "", Inst, BlockColors);
     }
   decline_rv_optimization:
     return false;
   }
   case ARCInstKind::InitWeak: {
     // objc_initWeak(p, null) => *p = null
     CallInst *CI = cast<CallInst>(Inst);
     if (IsNullOrUndef(CI->getArgOperand(1))) {
bool ObjCARCContract::tryToPeepholeInstruction(
   }
 }
 
 //===----------------------------------------------------------------------===//
 //                              Top Level Driver
 //===----------------------------------------------------------------------===//
 
 bool ObjCARCContract::init(Module &M) {
-  // If nothing in the Module uses ARC, don't do anything.
-  Run = ModuleHasARC(M);
-  if (!Run)
-    return false;
-
   EP.init(&M);
 
   // Initialize RVInstMarker.
-  const char *MarkerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
-  RVInstMarker = dyn_cast_or_null<MDString>(M.getModuleFlag(MarkerKey));
+  RVInstMarker = getRVInstMarker(M);
 
   return false;
 }
 
 bool ObjCARCContract::run(Function &F, AAResults *A, DominatorTree *D) {
   if (!EnableARCOpts)
     return false;
 
-  // If nothing in the Module uses ARC, don't do anything.
-  if (!Run)
-    return false;
-
-  Changed = false;
+  Changed = CFGChanged = false;
   AA = A;
   DT = D;
   PA.setAA(A);
+  BundledRetainClaimRVs BRV(EP, true);
+  BundledInsts = &BRV;
+
+  std::pair<bool, bool> R = BundledInsts->insertAfterInvokes(F, DT);
+  Changed |= R.first;
+  CFGChanged |= R.second;
 
   DenseMap<BasicBlock *, ColorVector> BlockColors;
   if (F.hasPersonalityFn() &&
       isScopedEHPersonality(classifyEHPersonality(F.getPersonalityFn())))
     BlockColors = colorEHFunclets(F);
 
   LLVM_DEBUG(llvm::dbgs() << "**** ObjCARC Contract ****\n");
 
   // Track whether it's ok to mark objc_storeStrong calls with the "tail"
   // keyword. Be conservative if the function has variadic arguments.
   // It seems that functions which "return twice" are also unsafe for the
   // "tail" argument, because they are setjmp, which could need to
   // return to an earlier stack state.
   bool TailOkForStoreStrongs =
       !F.isVarArg() && !F.callsFunctionThatReturnsTwice();
 
   // For ObjC library calls which return their argument, replace uses of the
   // argument with uses of the call return value, if it dominates the use. This
   // reduces register pressure.
   for (inst_iterator I = inst_begin(&F), E = inst_end(&F); I != E;) {
     Instruction *Inst = &*I++;
 
     LLVM_DEBUG(dbgs() << "Visiting: " << *Inst << "\n");
 
+    if (auto *CI = dyn_cast<CallInst>(Inst))
+      if (objcarc::hasRetainRVOrClaimRVAttr(CI)) {
+        BundledInsts->insertRVCallWithColors(&*I, CI, BlockColors);
+        --I;
+        Changed = true;
+      }
+
     // First try to peephole Inst. If there is nothing further we can do in
     // terms of undoing objc-arc-expand, process the next inst.
     if (tryToPeepholeInstruction(F, Inst, I, TailOkForStoreStrongs,
                                  BlockColors))
       continue;
 
     // Otherwise, try to undo objc-arc-expand.
 

 INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_END(ObjCARCContractLegacyPass, "objc-arc-contract",
                     "ObjC ARC contraction", false, false)
 
 void ObjCARCContractLegacyPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<AAResultsWrapperPass>();
   AU.addRequired<DominatorTreeWrapperPass>();
-  AU.setPreservesCFG();
 }
 
 Pass *llvm::createObjCARCContractPass() {
   return new ObjCARCContractLegacyPass();
 }
 
 bool ObjCARCContractLegacyPass::doInitialization(Module &M) {
   return OCARCC.init(M);
 }
 
 bool ObjCARCContractLegacyPass::runOnFunction(Function &F) {
   auto *AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
   auto *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
   return OCARCC.run(F, AA, DT);
 }
 
 PreservedAnalyses ObjCARCContractPass::run(Function &F,
                                            FunctionAnalysisManager &AM) {
   ObjCARCContract OCAC;
   OCAC.init(*F.getParent());
 
   bool Changed = OCAC.run(F, &AM.getResult<AAManager>(F),
                           &AM.getResult<DominatorTreeAnalysis>(F));
+  bool CFGChanged = OCAC.hasCFGChanged();
   if (Changed) {
     PreservedAnalyses PA;
+    if (!CFGChanged)
       PA.preserveSet<CFGAnalyses>();
     return PA;
   }
   return PreservedAnalyses::all();
 }

llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp

 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/Analysis/ObjCARCAliasAnalysis.h"
 #include "llvm/Analysis/ObjCARCAnalysisUtils.h"
 #include "llvm/Analysis/ObjCARCInstKind.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/InstIterator.h"
raw_ostream &llvm::operator<<(raw_ostream &OS, BBState &BBInfo) {
   return OS;
 }
 
 namespace {
 
   /// The main ARC optimization pass.
 class ObjCARCOpt {
   bool Changed;
+  bool CFGChanged;
   ProvenanceAnalysis PA;
 
   /// A cache of references to runtime entry point constants.
   ARCRuntimeEntryPoints EP;
 
   /// A cache of MDKinds that can be passed into other functions to propagate
   /// MDKind identifiers.
   ARCMDKindCache MDKindCache;
 
-  /// A flag indicating whether this optimization pass should run.
-  bool Run;
+  BundledRetainClaimRVs *BundledInsts = nullptr;
 
   /// A flag indicating whether the optimization that removes or moves
   /// retain/release pairs should be performed.
   bool DisableRetainReleasePairing = false;
 
   /// Flags which determine whether each of the interesting runtime functions
   /// is in fact used in the current function.
   unsigned UsedInThisFunction;

 #ifndef NDEBUG
   void GatherStatistics(Function &F, bool AfterOptimization = false);
 #endif
 
   public:
     void init(Module &M);
     bool run(Function &F, AAResults &AA);
     void releaseMemory();
+    bool hasCFGChanged() const { return CFGChanged; }
 };
 
 /// The main ARC optimization pass.
 class ObjCARCOptLegacyPass : public FunctionPass {
 public:
   ObjCARCOptLegacyPass() : FunctionPass(ID) {
     initializeObjCARCOptLegacyPassPass(*PassRegistry::getPassRegistry());
   }

 INITIALIZE_PASS_END(ObjCARCOptLegacyPass, "objc-arc", "ObjC ARC optimization",
                     false, false)
 
 Pass *llvm::createObjCARCOptPass() { return new ObjCARCOptLegacyPass(); }
 
 void ObjCARCOptLegacyPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<ObjCARCAAWrapperPass>();
   AU.addRequired<AAResultsWrapperPass>();
-  // ARC optimization doesn't currently split critical edges.
-  AU.setPreservesCFG();
 }
 
 /// Turn objc_retainAutoreleasedReturnValue into objc_retain if the operand is
 /// not a return value.
 bool
 ObjCARCOpt::OptimizeRetainRVCall(Function &F, Instruction *RetainRV) {
   // Check for the argument being from an immediately preceding call or invoke.
   const Value *Arg = GetArgRCIdentityRoot(RetainRV);
if (Call->getParent() == RetainRV->getParent()) {
         while (IsNoopInstruction(&*I))
           ++I;
         if (&*I == RetainRV)
           return false;
       }
     }
   }
 
+  assert(!BundledInsts->contains(RetainRV) &&
+         "a bundled retainRV's argument should be a call");
+
   // Turn it to a plain objc_retain.
   Changed = true;
   ++NumPeeps;
 
   LLVM_DEBUG(dbgs() << "Transforming objc_retainAutoreleasedReturnValue => "
                        "objc_retain since the operand is not a return value.\n"
                        "Old = "
                     << *RetainRV << "\n");
 
   Function *NewDecl = EP.get(ARCRuntimeEntryPointKind::Retain);
   cast<CallInst>(RetainRV)->setCalledFunction(NewDecl);
 
   LLVM_DEBUG(dbgs() << "New = " << *RetainRV << "\n");
 
   return false;
 }
 
 bool ObjCARCOpt::OptimizeInlinedAutoreleaseRVCall(
     Function &F, DenseMap<BasicBlock *, ColorVector> &BlockColors,
     Instruction *Inst, const Value *&Arg, ARCInstKind Class,
     Instruction *AutoreleaseRV, const Value *&AutoreleaseRVArg) {
+  if (BundledInsts->contains(Inst))
+    return false;
+
   // Must be in the same basic block.
   assert(Inst->getParent() == AutoreleaseRV->getParent());
 
   // Must operate on the same root.
   Arg = GetArgRCIdentityRoot(Inst);
   AutoreleaseRVArg = GetArgRCIdentityRoot(AutoreleaseRV);
   if (Arg != AutoreleaseRVArg) {
     // If there isn't an exact match, check if we have equivalent PHIs.
if (!CB)
       return true;
     return CB->getIntrinsicID() != Intrinsic::not_intrinsic;
   };
 
   // Visit all objc_* calls in F.
   for (inst_iterator I = inst_begin(&F), E = inst_end(&F); I != E; ) {
     Instruction *Inst = &*I++;
 
+    if (auto *CI = dyn_cast<CallInst>(Inst))
+      if (objcarc::hasRetainRVOrClaimRVAttr(CI)) {
+        BundledInsts->insertRVCall(&*I, CI);
+        Changed = true;
+      }
+
     ARCInstKind Class = GetBasicARCInstKind(Inst);
 
     // Skip this loop if this instruction isn't itself an ARC intrinsic.
     const Value *Arg = nullptr;
     switch (Class) {
     default:
       optimizeDelayedAutoreleaseRV();
       break;

 void ObjCARCOpt::OptimizeIndividualCallImpl(
     Function &F, DenseMap<BasicBlock *, ColorVector> &BlockColors,
     Instruction *Inst, ARCInstKind Class, const Value *Arg) {
   LLVM_DEBUG(dbgs() << "Visiting: Class: " << Class << "; " << *Inst << "\n");
 
   // We can delete this call if it takes an inert value.
   SmallPtrSet<Value *, 1> VisitedPhis;
 
+  if (BundledInsts->contains(Inst)) {
+    UsedInThisFunction |= 1 << unsigned(Class);
+    return;
+  }
+
   if (IsNoopOnGlobal(Class))
     if (isInertARCValue(Inst->getOperand(0), VisitedPhis)) {
       if (!Inst->getType()->isVoidTy())
         Inst->replaceAllUsesWith(Inst->getOperand(0));
       Inst->eraseFromParent();
       Changed = true;
       return;
     }
ObjCARCOpt::VisitInstructionTopDown(Instruction *Inst,
   // pointer being tracked.
   for (auto MI = MyStates.top_down_ptr_begin(),
             ME = MyStates.top_down_ptr_end();
        MI != ME; ++MI) {
     const Value *Ptr = MI->first;
     if (Ptr == Arg)
       continue; // Handled above.
     TopDownPtrState &S = MI->second;
-    if (S.HandlePotentialAlterRefCount(Inst, Ptr, PA, Class))
+    if (S.HandlePotentialAlterRefCount(Inst, Ptr, PA, Class, *BundledInsts))
       continue;
 
     S.HandlePotentialUse(Inst, Ptr, PA, Class);
   }
 
   return NestingDetected;
 }
 
if (!Call ||
          GetBasicARCInstKind(Autorelease) == ARCInstKind::AutoreleaseRV))
       continue;
 
     // If so, we can zap the retain and autorelease.
     Changed = true;
     ++NumRets;
     LLVM_DEBUG(dbgs() << "Erasing: " << *Retain << "\nErasing: " << *Autorelease
                       << "\n");
-    EraseInstruction(Retain);
+    BundledInsts->eraseInst(Retain);
     EraseInstruction(Autorelease);
   }
 }
 
 #ifndef NDEBUG
 void
 ObjCARCOpt::GatherStatistics(Function &F, bool AfterOptimization) {
   Statistic &NumRetains =
ObjCARCOpt::GatherStatistics(Function &F, bool AfterOptimization) {
   }
 }
 #endif
 
 void ObjCARCOpt::init(Module &M) {
   if (!EnableARCOpts)
     return;
 
-  // If nothing in the Module uses ARC, don't do anything.
-  Run = ModuleHasARC(M);
-  if (!Run)
-    return;
-
   // Intuitively, objc_retain and others are nocapture, however in practice
   // they are not, because they return their argument value. And objc_release
   // calls finalizers which can have arbitrary side effects.
   MDKindCache.init(&M);
 
   // Initialize our runtime entry point cache.
   EP.init(&M);
 }
 
 bool ObjCARCOpt::run(Function &F, AAResults &AA) {
   if (!EnableARCOpts)
     return false;
 
-  // If nothing in the Module uses ARC, don't do anything.
-  if (!Run)
-    return false;
-
-  Changed = false;
+  Changed = CFGChanged = false;
+  BundledRetainClaimRVs BRV(EP, false);
+  BundledInsts = &BRV;
 
   LLVM_DEBUG(dbgs() << "<<< ObjCARCOpt: Visiting Function: " << F.getName()
                     << " >>>"
                        "\n");
 
+  std::pair<bool, bool> R = BundledInsts->insertAfterInvokes(F, nullptr);
+  Changed |= R.first;
+  CFGChanged |= R.second;
+
   PA.setAA(&AA);
 
 #ifndef NDEBUG
   if (AreStatisticsEnabled()) {
     GatherStatistics(F, false);
   }
 #endif
 

 ///
 
 PreservedAnalyses ObjCARCOptPass::run(Function &F,
                                       FunctionAnalysisManager &AM) {
   ObjCARCOpt OCAO;
   OCAO.init(*F.getParent());
 
   bool Changed = OCAO.run(F, AM.getResult<AAManager>(F));
+  bool CFGChanged = OCAO.hasCFGChanged();
   if (Changed) {
     PreservedAnalyses PA;
+    if (!CFGChanged)
       PA.preserveSet<CFGAnalyses>();
     return PA;
   }
   return PreservedAnalyses::all();
 }

llvm/lib/Transforms/ObjCARC/PtrState.h

 class Instruction;
 class MDNode;
 class raw_ostream;
 class Value;
 
 namespace objcarc {
 
 class ARCMDKindCache;
+class BundledRetainClaimRVs;
 class ProvenanceAnalysis;
 
 /// \enum Sequence
 ///
 /// A sequence of states that a pointer may go through in which an
 /// objc_retain and objc_release are actually needed.
 enum Sequence {
   S_None,
struct TopDownPtrState : PtrState {
   /// release. Modifies state appropriately to reflect that the matching
   /// occurred.
   bool MatchWithRelease(ARCMDKindCache &Cache, Instruction *Release);
 
   void HandlePotentialUse(Instruction *Inst, const Value *Ptr,
                           ProvenanceAnalysis &PA, ARCInstKind Class);
 
   bool HandlePotentialAlterRefCount(Instruction *Inst, const Value *Ptr,
-                                    ProvenanceAnalysis &PA, ARCInstKind Class);
+                                    ProvenanceAnalysis &PA, ARCInstKind Class,
+                                    const BundledRetainClaimRVs &BundledRVs);
 };
 
 } // end namespace objcarc
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TRANSFORMS_OBJCARC_PTRSTATE_H

llvm/lib/Transforms/ObjCARC/PtrState.cpp

 //===- PtrState.cpp -------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "PtrState.h"
 #include "DependencyAnalysis.h"
 #include "ObjCARC.h"
 #include "llvm/Analysis/ObjCARCAnalysisUtils.h"
 #include "llvm/Analysis/ObjCARCInstKind.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
auto SetSeqAndInsertReverseInsertPt = [&](Sequence NewSeq){
     } else {
       InsertAfter = std::next(Inst->getIterator());
     }
 
     if (InsertAfter != BB->end())
       InsertAfter = skipDebugIntrinsics(InsertAfter);
 
     InsertReverseInsertPt(&*InsertAfter);
+
+    // Don't insert anything between a call/invoke annotated with
+    // retainRV/claimRV and the retainRV/claimRV call that uses the call result.
+    if (auto *CB = dyn_cast<CallBase>(Inst))
+      if (objcarc::hasRetainRVOrClaimRVAttr(CB))
+        SetCFGHazardAfflicted(true);
   };
 
   // Check for possible direct uses.
   switch (GetSeq()) {
   case S_Release:
   case S_MovableRelease:
     if (CanUse(Inst, Ptr, PA, Class)) {
       LLVM_DEBUG(dbgs() << "            CanUse: Seq: " << GetSeq() << "; "
bool TopDownPtrState::MatchWithRelease(ARCMDKindCache &Cache,
   case S_Stop:
   case S_Release:
   case S_MovableRelease:
     llvm_unreachable("top-down pointer in bottom up state!");
   }
   llvm_unreachable("Sequence unknown enum value");
 }
 
-bool TopDownPtrState::HandlePotentialAlterRefCount(Instruction *Inst,
-                                                   const Value *Ptr,
-                                                   ProvenanceAnalysis &PA,
-                                                   ARCInstKind Class) {
+bool TopDownPtrState::HandlePotentialAlterRefCount(
+    Instruction *Inst, const Value *Ptr, ProvenanceAnalysis &PA,
+    ARCInstKind Class, const BundledRetainClaimRVs &BundledRVs) {
   // Check for possible releases. Treat clang.arc.use as a releasing instruction
   // to prevent sinking a retain past it.
   if (!CanDecrementRefCount(Inst, Ptr, PA, Class) &&
       Class != ARCInstKind::IntrinsicUser)
     return false;
 
   LLVM_DEBUG(dbgs() << "            CanAlterRefCount: Seq: " << GetSeq() << "; "
                     << *Ptr << "\n");
   ClearKnownPositiveRefCount();
   switch (GetSeq()) {
   case S_Retain:
     SetSeq(S_CanRelease);
     assert(!HasReverseInsertPts());
     InsertReverseInsertPt(Inst);
 
+    // Don't insert anything between a call/invoke annotated with
+    // retainRV/claimRV and the retainRV/claimRV call that uses the call result.
+    if (BundledRVs.contains(Inst))
+      SetCFGHazardAfflicted(true);
+
     // One call can't cause a transition from S_Retain to S_CanRelease
     // and S_CanRelease to S_Use. If we've made the first transition,
     // we're done.
     return true;
   case S_Use:
   case S_CanRelease:
   case S_None:
     return false;

llvm/lib/Transforms/Utils/InlineFunction.cpp

 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/ObjCARCAnalysisUtils.h"
+#include "llvm/Analysis/ObjCARCRVAttr.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
-#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DIBuilder.h"

 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Transforms/Utils/AssumeBundleBuilder.h"
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <iterator>
 #include <limits>
 #include <string>
 #include <utility>
for (BasicBlock &BB : *Callee)
       // No need to update the callsite if it is pruned during inlining.
       if (!VMap || VMap->count(&BB))
         for (Instruction &I : BB)
           if (CallInst *CI = dyn_cast<CallInst>(&I))
             CI->updateProfWeight(newEntryCount, priorEntryCount);
   }
 }
 
+static void
+insertRetainOrClaimRVCalls(CallBase &CB,
+                           const SmallVectorImpl<ReturnInst *> &Returns) {
+  Module *Mod = CB.getParent()->getParent()->getParent();
+  bool IsRetainRV = objcarc::hasRetainRVAttr(&CB), IsClaimRV = !IsRetainRV;
+
+  for (auto *RI : Returns) {
+    Value *RetOpnd = llvm::objcarc::GetRCIdentityRoot(RI->getOperand(0));
+    BasicBlock::reverse_iterator I = ++(RI->getIterator().getReverse());
+    BasicBlock::reverse_iterator EI = RI->getParent()->rend();
+    bool InsertRetainCall = IsRetainRV;
+    IRBuilder<> Builder(RI->getContext());
+
+    // Walk backwards through the basic block looking for either a matching
+    // autoreleaseRV call or an unannotated call.
+    for (; I != EI;) {
+      auto CurI = I++;
+
+      // Ignore casts.
+      if (isa<CastInst>(*CurI))
+        continue;
+
+      if (auto *II = dyn_cast<IntrinsicInst>(&*CurI)) {
+        if (II->getIntrinsicID() == Intrinsic::objc_autoreleaseReturnValue &&
+            II->hasNUses(0) &&
+            llvm::objcarc::GetRCIdentityRoot(II->getOperand(0)) == RetOpnd) {
+          // If we've found a matching authoreleaseRV call:
+          // - If the call is annotated with claimRV, insert a call to
+          //   objc_release and erase the autoreleaseRV call.
+          // - If the call is annotated with retainRV, just erase the
+          //   autoreleaseRV call.
+          if (IsClaimRV) {
+            Builder.SetInsertPoint(II);
+            Function *IFn =
+                Intrinsic::getDeclaration(Mod, Intrinsic::objc_release);
+            Value *BC =
+                Builder.CreateBitCast(RetOpnd, IFn->getArg(0)->getType());
+            Builder.CreateCall(IFn, BC, "");
+          }
+          II->eraseFromParent();
+          InsertRetainCall = false;
+        }
+      } else if (auto *CI = dyn_cast<CallInst>(&*CurI)) {
+        if (llvm::objcarc::GetRCIdentityRoot(CI) == RetOpnd &&
+            !objcarc::hasRetainRVOrClaimRVAttr(CI)) {
+          // If we've found an unannotated call that defines RetOpnd, annotate
+          // the call with the attributes.
+          llvm::AttributeList AL = CI->getAttributes();
+          AL = AL.addAttribute(CI->getContext(), AttributeList::ReturnIndex,
+                               objcarc::getRVAttrKeyStr(),
+                               objcarc::getRVAttrValStr(IsRetainRV));
+          CI->setAttributes(AL);
+          InsertRetainCall = false;
+        }
+      }
+
+      break;
+    }
+
+    if (InsertRetainCall) {
+      // The call is annotated with retainRV and we've failed to find a matching
+      // autoreleaseRV or an annotated call in the callee. Emit a call to
+      // objc_retain.
+      Builder.SetInsertPoint(RI);
+      Function *IFn = Intrinsic::getDeclaration(Mod, Intrinsic::objc_retain);
+      Value *BC = Builder.CreateBitCast(RetOpnd, IFn->getArg(0)->getType());
+      Builder.CreateCall(IFn, BC, "");
+    }
+  }
+}
+
 /// This function inlines the called function into the basic block of the
 /// caller. This returns false if it is not possible to inline this call.
 /// The program is still in a well defined state if this occurs though.
 ///
 /// Note that this only does one level of inlining.  For example, if the
 /// instruction 'call B' is inlined, and 'B' calls 'C', then the call to 'C' now
 /// exists in the instruction stream.  Similarly this will inline a recursive
 /// function by one level.
Function::iterator FirstNewBlock;
     // (which can happen, e.g., because an argument was constant), but we'll be
     // happy with whatever the cloner can do.
     CloneAndPruneFunctionInto(Caller, CalledFunc, VMap,
                               /*ModuleLevelChanges=*/false, Returns, ".i",
                               &InlinedFunctionInfo, &CB);
     // Remember the first block that is newly cloned over.
     FirstNewBlock = LastBlock; ++FirstNewBlock;
 
+    // Insert retainRV/clainRV runtime calls.
+    if (objcarc::hasRetainRVOrClaimRVAttr(&CB))
+      insertRetainOrClaimRVCalls(CB, Returns);
+
     if (IFI.CallerBFI != nullptr && IFI.CalleeBFI != nullptr)
       // Update the BFI of blocks cloned into the caller.
       updateCallerBFI(OrigBB, VMap, IFI.CallerBFI, IFI.CalleeBFI,
                       CalledFunc->front());
 
     updateCallProfile(CalledFunc, VMap, CalledFunc->getEntryCount(), CB,
                       IFI.PSI, IFI.CallerBFI);
 

llvm/test/CodeGen/AArch64/call-rv-marker.ll

 define dso_local i8* @rv_marker_1() {
 ; CHECK-LABEL:    rv_marker_1:
 ; CHECK:           .cfi_offset w30, -16
 ; CHECK-NEXT:      bl foo1
 ; SELDAG-NEXT:     mov x29, x29
 ; GISEL-NOT:       mov x29, x29
 ;
 entry:
-  %call = call "rv_marker" i8* @foo1()
+  %call = call "clang.arc.rv"="retain" i8* @foo1()
   ret i8* %call
 }
 
 define dso_local void @rv_marker_2_select(i32 %c) {
 ; CHECK-LABEL: rv_marker_2_select:
 ; SELDAG:        cinc  w0, w8, eq
 ; GISEL:         csinc w0, w8, wzr, eq
 ; CHECK-NEXT:    bl  foo0
 ; SELDAG-NEXT:   mov x29, x29
 ; CHECK-NEXT:    ldr x30, [sp], #16
 ; CHECK-NEXT:    b  foo2
 ;
 entry:
   %tobool.not = icmp eq i32 %c, 0
   %.sink = select i1 %tobool.not, i32 2, i32 1
-  %call1 = call "rv_marker" i8* @foo0(i32 %.sink)
+  %call1 = call "clang.arc.rv"="retain" i8* @foo0(i32 %.sink)
   tail call void @foo2(i8* %call1)
   ret void
 }
 
 define dso_local void @rv_marker_3() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 ; CHECK-LABEL: rv_marker_3
 ; CHECK:         .cfi_offset w30, -32
 ; CHECK-NEXT:    bl  foo1
 ; SELDAG-NEXT:   mov x29, x29
 ;
 entry:
-  %call = call "rv_marker" i8* @foo1()
+  %call = call "clang.arc.rv"="retain" i8* @foo1()
   invoke void @objc_object(i8* %call) #5
           to label %invoke.cont unwind label %lpad
 
 invoke.cont:                                      ; preds = %entry
   tail call void @llvm.objc.release(i8* %call)
   ret void
 
 lpad:                                             ; preds = %entry

 ; CHECK-NEXT:     bl  foo1
 ; SELDAG-NEXT:    mov x29, x29
 ; CHECK-NEXT: .Ltmp4:
 ;
 entry:
   %s = alloca %struct.S, align 1
   %0 = getelementptr inbounds %struct.S, %struct.S* %s, i64 0, i32 0
   call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %0) #2
-  %call = invoke "rv_marker" i8* @foo1()
+  %call = invoke "clang.arc.rv"="retain" i8* @foo1()
           to label %invoke.cont unwind label %lpad
 
 invoke.cont:                                      ; preds = %entry
   invoke void @objc_object(i8* %call) #5
           to label %invoke.cont2 unwind label %lpad1
 
 invoke.cont2:                                     ; preds = %invoke.cont
   tail call void @llvm.objc.release(i8* %call)

 ; CHECK-LABEL: rv_marker_5_indirect_call
 ; CHECK:         ldr [[ADDR:x[0-9]+]], [
 ; CHECK-NEXT:    blr [[ADDR]]
 ; SLEDAG-NEXT:   mov x29, x29
 ; GISEL-NOT:     mov x29, x29
 ;
 entry:
   %0 = load i8* ()*, i8* ()** @fptr, align 8
-  %call = call "rv_marker" i8* %0()
+  %call = call "clang.arc.rv"="retain" i8* %0()
   tail call void @foo2(i8* %call)
   ret i8* %call
 }
 
 declare void @foo(i64, i64, i64)
 
 define dso_local void @rv_marker_multiarg(i64 %a, i64 %b, i64 %c) {
 ; CHECK-LABEL: rv_marker_multiarg
 ; CHECK:        mov [[TMP:x[0-9]+]], x0
 ; CHECK-NEXT:   mov x0, x2
 ; CHECK-NEXT:   mov x2, [[TMP]]
 ; CHECK-NEXT:   bl  foo
 ; SELDAG-NEXT:  mov x29, x29
 ; GISEL-NOT:    mov x29, x29
-  call "rv_marker" void @foo(i64 %c, i64 %b, i64 %a)
+  call "clang.arc.rv"="retain" void @foo(i64 %c, i64 %b, i64 %a)
   ret void
 }
 
 declare i32 @__gxx_personality_v0(...)

llvm/test/Transforms/DeadArgElim/deadretval.ll

-; RUN: opt < %s -deadargelim -S | not grep DEAD
+; RUN: opt < %s -deadargelim -S | FileCheck %s
+
+@g0 = global i8 0, align 8
+
+; CHECK-NOT: DEAD
 
 ; Dead arg only used by dead retval
 define internal i32 @test(i32 %DEADARG) {
         ret i32 %DEADARG
 }
 
 define i32 @test2(i32 %A) {
         %DEAD = call i32 @test( i32 %A )                ; <i32> [#uses=0]
         ret i32 123
 }
 
 define i32 @test3() {
         %X = call i32 @test2( i32 3232 )                ; <i32> [#uses=1]
         %Y = add i32 %X, -123           ; <i32> [#uses=1]
         ret i32 %Y
 }
 
+; The callee function's return type shouldn't be changed if there is a call
+; annotated with "clang.arc.rv"="retain" or "clang.arc.rv"="claim".
+
+; CHECK-LABEL: define internal i8* @callee4()
+
+define internal i8* @callee4(i8* %a0) {
+  ret i8* @g0;
+}
+
+; CHECK-LABEL: define i8* @test4(
+; CHECK: tail call "clang.arc.rv"="retain" i8* @callee4()
+
+define i8* @test4() {
+  %call = tail call "clang.arc.rv"="retain" i8* @callee4(i8* @g0)
+  ret i8* @g0
+}

llvm/test/Transforms/Inline/inline-retainRV-call.ll

+; RUN: opt < %s -inline -S | FileCheck %s
+
+@g0 = global i8* null, align 8
+declare i8* @foo0()
+
+define i8* @callee0_autoreleaseRV() {
+  %call = call "clang.arc.rv"="retain" i8* @foo0()
+  %1 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %call)
+  ret i8* %call
+}
+
+; CHECK-LABEL: define void @test0_autoreleaseRV(
+; CHECK: call "clang.arc.rv"="retain" i8* @foo0()
+
+define void @test0_autoreleaseRV() {
+  %call = call "clang.arc.rv"="retain" i8* @callee0_autoreleaseRV()
+  ret void
+}
+
+; CHECK-LABEL: define void @test0_claimRV_autoreleaseRV(
+; CHECK: %[[CALL:.*]] = call "clang.arc.rv"="retain" i8* @foo0()
+; CHECK: call void @llvm.objc.release(i8* %[[CALL]])
+; CHECK-NEXT: ret void
+
+define void @test0_claimRV_autoreleaseRV() {
+  %call = call "clang.arc.rv"="claim" i8* @callee0_autoreleaseRV()
+  ret void
+}
+
+; CHECK-LABEL: define void @test1_autoreleaseRV(
+; CHECK: invoke "clang.arc.rv"="retain" i8* @foo0()
+
+define void @test1_autoreleaseRV() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %call = invoke "clang.arc.rv"="retain" i8* @callee0_autoreleaseRV()
+          to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+          cleanup
+  resume { i8*, i32 } undef
+}
+
+; CHECK-LABEL: define void @test1_claimRV_autoreleaseRV(
+; CHECK: %[[INVOKE:.*]] = invoke "clang.arc.rv"="retain" i8* @foo0()
+; CHECK: call void @llvm.objc.release(i8* %[[INVOKE]])
+; CHECK-NEXT: br
+
+define void @test1_claimRV_autoreleaseRV() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %call = invoke "clang.arc.rv"="claim" i8* @callee0_autoreleaseRV()
+          to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+          cleanup
+  resume { i8*, i32 } undef
+}
+
+define i8* @callee1_no_autoreleaseRV() {
+  %call = call i8* @foo0()
+  ret i8* %call
+}
+
+; CHECK-LABEL: define void @test2_no_autoreleaseRV(
+; CHECK: call "clang.arc.rv"="retain" i8* @foo0()
+; CHECK-NEXT: ret void
+
+define void @test2_no_autoreleaseRV() {
+  %call = call "clang.arc.rv"="retain" i8* @callee1_no_autoreleaseRV()
+  ret void
+}
+
+; CHECK-LABEL: define void @test2_claimRV_no_autoreleaseRV(
+; CHECK: call "clang.arc.rv"="claim" i8* @foo0()
+; CHECK-NEXT: ret void
+
+define void @test2_claimRV_no_autoreleaseRV() {
+  %call = call "clang.arc.rv"="claim" i8* @callee1_no_autoreleaseRV()
+  ret void
+}
+
+; CHECK-LABEL: define void @test3_no_autoreleaseRV(
+; CHECK: invoke "clang.arc.rv"="retain" i8* @foo0()
+
+define void @test3_no_autoreleaseRV() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %call = invoke "clang.arc.rv"="retain" i8* @callee1_no_autoreleaseRV()
+          to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+          cleanup
+  resume { i8*, i32 } undef
+}
+
+define i8* @callee2_nocall() {
+  %1 = load i8*, i8** @g0, align 8
+  ret i8* %1
+}
+
+; Check that a call to @llvm.objc.retain is inserted if there is no matching
+; autoreleaseRV call or a call.
+
+; CHECK-LABEL: define void @test4_nocall(
+; CHECK: %[[V0:.*]] = load i8*, i8** @g0,
+; CHECK-NEXT: call i8* @llvm.objc.retain(i8* %[[V0]])
+; CHECK-NEXT: ret void
+
+define void @test4_nocall() {
+  %call = call "clang.arc.rv"="retain" i8* @callee2_nocall()
+  ret void
+}
+
+; CHECK-LABEL: define void @test4_claimRV_nocall(
+; CHECK: %[[V0:.*]] = load i8*, i8** @g0,
+; CHECK-NEXT: ret void
+
+define void @test4_claimRV_nocall() {
+  %call = call "clang.arc.rv"="claim" i8* @callee2_nocall()
+  ret void
+}
+
+; Check that a call to @llvm.objc.retain is inserted if call to @foo already has
+; the attribute. I'm not sure this will happen in practice.
+
+define i8* @callee3_marker() {
+  %1 = call "clang.arc.rv"="retain" i8* @foo0()
+  ret i8* %1
+}
+
+; CHECK-LABEL: define void @test5(
+; CHECK: %[[V0:.*]] = call "clang.arc.rv"="retain" i8* @foo0()
+; CHECK-NEXT: call i8* @llvm.objc.retain(i8* %[[V0]])
+; CHECK-NEXT: ret void
+
+define void @test5() {
+  %call = call "clang.arc.rv"="retain" i8* @callee3_marker()
+  ret void
+}
+
+; Don't pair up an autoreleaseRV in the callee and an retainRV in the caller
+; if there is an instruction between the ret instruction and the call to
+; autoreleaseRV that isn't a cast instruction.
+
+define i8* @callee0_autoreleaseRV2() {
+  %call = call "clang.arc.rv"="retain" i8* @foo0()
+  %1 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %call)
+  store i8* null, i8** @g0
+  ret i8* %call
+}
+
+; CHECK-LABEL: define void @test6(
+; CHECK: %[[V0:.*]] = call "clang.arc.rv"="retain" i8* @foo0()
+; CHECK: call i8* @llvm.objc.autoreleaseReturnValue(i8* %[[V0]])
+; CHECK: store i8* null, i8** @g0, align 8
+; CHECK: call i8* @llvm.objc.retain(i8* %[[V0]])
+; CHECK-NEXT: ret void
+
+define void @test6() {
+  %call = call "clang.arc.rv"="retain" i8* @callee0_autoreleaseRV2()
+  ret void
+}
+
+declare i8* @llvm.objc.autoreleaseReturnValue(i8*)
+declare i32 @__gxx_personality_v0(...)

llvm/test/Transforms/ObjCARC/contract-marker-funclet.ll

 ; RUN: opt -mtriple=i686-unknown-windows-msvc -objc-arc-contract -S -o - %s | FileCheck %s
 
 ; Generated (and lightly modified and cleaned up) from the following source:
 ; id f();
 ; void g() {
 ;   try {
 ;     f();
 ;   } catch (...) {
 ;     f();
 ;   }
 ; }
 
+; CHECK-LABEL: define void @"\01?g@@YAXXZ"()
+; CHECK-LABEL: catch
+; CHECK: call void asm sideeffect "movl{{.*}}%ebp, %ebp{{.*}}", ""() [ "funclet"(token %1) ]
+
+; CHECK-LABEL: catch.1
+; CHECK: call void asm sideeffect "movl{{.*}}%ebp, %ebp{{.*}}", ""() [ "funclet"(token %1) ]
+
+; CHECK-LABEL: invoke.cont
+; CHECK: call void asm sideeffect "movl{{.*}}%ebp, %ebp{{.*}}", ""(){{$}}
+
 define void @"\01?g@@YAXXZ"() personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
 entry:
   %call = invoke i8* @"\01?f@@YAPAUobjc_object@@XZ"()
           to label %invoke.cont unwind label %catch.dispatch
 
 catch.dispatch:                                   ; preds = %entry
   %0 = catchswitch within none [label %catch] unwind to caller
 
catchret.dest:                                    ; preds = %catch.1
   ret void
 
 invoke.cont:                                      ; preds = %entry
   %4 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %call)
   call void @llvm.objc.release(i8* %4)
   ret void
 }
 
+; CHECK-LABEL: define dso_local void @"?test_attr_claimRV@@YAXXZ"()
+; CHECK: %[[CALL4:.*]] = notail call "clang.arc.rv"="claim" i8* @"?noexcept_func@@YAPAUobjc_object@@XZ"()
+; CHECK: call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* %[[CALL4]])
+
+; CHECK: %[[V1:.*]] = cleanuppad
+; CHECK: %[[CALL:.*]] = notail call "clang.arc.rv"="claim" i8* @"?noexcept_func@@YAPAUobjc_object@@XZ"() [ "funclet"(token %[[V1]]) ]
+; CHECK: call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* %[[CALL]]) [ "funclet"(token %[[V1]]) ]
+
+define dso_local void @"?test_attr_claimRV@@YAXXZ"() local_unnamed_addr #0 personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+  invoke void @"?foo@@YAXXZ"()
+          to label %invoke.cont unwind label %ehcleanup
+
+invoke.cont:                                      ; preds = %entry
+  %call.i4 = tail call "clang.arc.rv"="claim" i8* @"?noexcept_func@@YAPAUobjc_object@@XZ"() #2
+  ret void
+
+ehcleanup:                                        ; preds = %entry
+  %0 = cleanuppad within none []
+  %call.i = call "clang.arc.rv"="claim" i8* @"?noexcept_func@@YAPAUobjc_object@@XZ"() #2 [ "funclet"(token %0) ]
+  cleanupret from %0 unwind to caller
+}
+
 declare i8* @"\01?f@@YAPAUobjc_object@@XZ"()
 
 declare i32 @__CxxFrameHandler3(...)
 
+declare void @"?foo@@YAXXZ"()
+declare i8* @"?noexcept_func@@YAPAUobjc_object@@XZ"()
+
 declare dllimport i8* @llvm.objc.retainAutoreleasedReturnValue(i8*)
+declare i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8*)
 
 declare dllimport void @llvm.objc.release(i8*)
 
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"movl\09%ebp, %ebp\09\09// marker for objc_retainAutoreleaseReturnValue"}
-
-; CHECK-LABEL: catch
-; CHECK: call void asm sideeffect "movl{{.*}}%ebp, %ebp{{.*}}", ""() [ "funclet"(token %1) ]
-
-; CHECK-LABEL: catch.1
-; CHECK: call void asm sideeffect "movl{{.*}}%ebp, %ebp{{.*}}", ""() [ "funclet"(token %1) ]
-
-; CHECK-LABEL: invoke.cont
-; CHECK: call void asm sideeffect "movl{{.*}}%ebp, %ebp{{.*}}", ""(){{$}}

llvm/test/Transforms/ObjCARC/contract-rv-attr.ll

+; RUN: opt -objc-arc-contract -S < %s | FileCheck %s
+; RUN: opt -passes=objc-arc-contract -S < %s | FileCheck %s
+
+; CHECK-LABEL: define void @test0() {
+; CHECK: %[[CALL:.*]] = notail call "clang.arc.rv"="retain" i8* @foo()
+; CHECK: call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL]])
+
+define void @test0() {
+  %call1 = call "clang.arc.rv"="retain" i8* @foo()
+  ret void
+}
+
+; CHECK-LABEL: define void @test1() {
+; CHECK: %[[CALL:.*]] = notail call "clang.arc.rv"="claim" i8* @foo()
+; CHECK: call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* %[[CALL]])
+
+define void @test1() {
+  %call1 = call "clang.arc.rv"="claim" i8* @foo()
+  ret void
+}
+
+; CHECK-LABEL:define i8* @test2(
+; CHECK: %[[CALL1:.*]] = invoke "clang.arc.rv"="retain" i8* @foo()
+
+; CHECK: %[[V0:.*]] = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL1]])
+; CHECK-NEXT: br
+
+; CHECK: %[[CALL3:.*]] = invoke "clang.arc.rv"="retain" i8* @foo()
+
+; CHECK: %[[V2:.*]] = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL3]])
+; CHECK-NEXT: br
+
+; CHECK: %[[RETVAL:.*]] = phi i8* [ %[[V0]], {{.*}} ], [ %[[V2]], {{.*}} ]
+; CHECK: ret i8* %[[RETVAL]]
+
+define i8* @test2(i1 zeroext %b) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  br i1 %b, label %if.then, label %if.end
+
+if.then:
+  %call1 = invoke "clang.arc.rv"="retain" i8* @foo()
+          to label %cleanup unwind label %lpad
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+          cleanup
+  resume { i8*, i32 } undef
+
+if.end:
+  %call3 = invoke "clang.arc.rv"="retain" i8* @foo()
+          to label %cleanup unwind label %lpad
+
+cleanup:
+  %retval.0 = phi i8* [ %call1, %if.then ], [ %call3, %if.end ]
+  ret i8* %retval.0
+}
+
+declare i8* @foo()
+declare i32 @__gxx_personality_v0(...)
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov\09fp, fp\09\09// marker for objc_retainAutoreleaseReturnValue"}

llvm/test/Transforms/ObjCARC/rv.ll

bb0:
   ret i8* %v0
 bb1:
   %v3 = tail call i8* @returner()
   %v4 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %v3)
   %v5 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %v3)
   ret i8* %v3
 }
 
+; Remove attributes and the autoreleaseRV call if the call is a tail call.
+
+; CHECK-LABEL: define i8* @test31(
+; CHECK: %[[CALL:.*]] = tail call i8* @returner()
+; CHECK: ret i8* %[[CALL]]
+
+define i8* @test31() {
+  %call = tail call "clang.arc.rv"="retain" i8* @returner()
+  %1 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %call)
+  ret i8* %1
+}
+
+; CHECK-LABEL: define i8* @test32(
+; CHECK: %[[CALL:.*]] = call "clang.arc.rv"="retain" i8* @returner()
+; CHECK: call i8* @llvm.objc.autoreleaseReturnValue(i8* %[[CALL]])
+
+define i8* @test32() {
+  %call = call "clang.arc.rv"="retain" i8* @returner()
+  %1 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %call)
+  ret i8* %1
+}
+
 !0 = !{}
 
 ; CHECK: attributes [[NUW]] = { nounwind }