From 92bae8f139b8de18e36e03a1963a915b5c3d7ec6 Mon Sep 17 00:00:00 2001 From: Ahmed Bougacha Date: Tue, 12 Apr 2022 13:17:49 -0700 Subject: [PATCH] [ObjCARC] Tolerate missing attachedcall op by ignoring the bundle. In some situations (swiftc bitcode) we end up running the objc-arc-contract pass twice, which is not supposed to occur. This was fine until relatively recently, where the clang.arc.attachedcall operand bundle support caused the second run to crash: when emitting the calls, the pass was assuming that the bundle always had an operand, which is not true in 5.6. That will be true in the future, but that's a massive change. For now, try to workaround the issue by ignoring the nullary bundles, and not emitting the call. The pass will get a little confused, but the semantics are preserved by the presence of the intrinsic call later on (emitted by the first run of the pass). However, one difference is that we're now going to emit a redundant inline-asm marker, since we're not able to eliminate the (standalone) intrinsic anymore, as we don't remember emitting it in the pass. That costs us an extra nop, which is okay. rdar://90366906 --- llvm/lib/Transforms/ObjCARC/ObjCARC.cpp | 12 ++++--- .../Transforms/ObjCARC/ObjCARCContract.cpp | 4 +-- .../ObjCARC/contract-rv-attr-empty-bundle.ll | 32 +++++++++++++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 llvm/test/Transforms/ObjCARC/contract-rv-attr-empty-bundle.ll diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARC.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARC.cpp index 1ca6ddabac5bc..dcc47582cf520 100644 --- a/llvm/lib/Transforms/ObjCARC/ObjCARC.cpp +++ b/llvm/lib/Transforms/ObjCARC/ObjCARC.cpp @@ -103,12 +103,16 @@ CallInst *BundledRetainClaimRVs::insertRVCallWithColors( Instruction *InsertPt, CallBase *AnnotatedCall, const DenseMap &BlockColors) { IRBuilder<> Builder(InsertPt); - Function *Func = *objcarc::getAttachedARCFunction(AnnotatedCall); - assert(Func && "operand isn't a Function"); - Type *ParamTy = Func->getArg(0)->getType(); + Optional Func = objcarc::getAttachedARCFunction(AnnotatedCall); + // This might be a call with an empty bundle, because contract already ran. + // In that case, we don't have to insert another call. + if (!Func) + return nullptr; + + Type *ParamTy = (*Func)->getArg(0)->getType(); Value *CallArg = Builder.CreateBitCast(AnnotatedCall, ParamTy); auto *Call = - createCallInstWithColors(Func, CallArg, "", InsertPt, BlockColors); + createCallInstWithColors(*Func, CallArg, "", InsertPt, BlockColors); RVCalls[Call] = AnnotatedCall; return Call; } diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp index c2ed94e8e1f62..a0738f45594af 100644 --- a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp +++ b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp @@ -580,8 +580,8 @@ bool ObjCARCContract::run(Function &F, AAResults *A, DominatorTree *D) { if (auto *CI = dyn_cast(Inst)) if (objcarc::hasAttachedCallOpBundle(CI)) { - BundledInsts->insertRVCallWithColors(&*I, CI, BlockColors); - --I; + if (BundledInsts->insertRVCallWithColors(&*I, CI, BlockColors)) + --I; Changed = true; } diff --git a/llvm/test/Transforms/ObjCARC/contract-rv-attr-empty-bundle.ll b/llvm/test/Transforms/ObjCARC/contract-rv-attr-empty-bundle.ll new file mode 100644 index 0000000000000..1f6ffdbf74e0b --- /dev/null +++ b/llvm/test/Transforms/ObjCARC/contract-rv-attr-empty-bundle.ll @@ -0,0 +1,32 @@ +; 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 i8* @foo() [ "clang.arc.attachedcall"() ] +; CHECK-NEXT: call void asm sideeffect "mov\09fp, fp\09\09// marker for objc_retainAutoreleaseReturnValue", ""() +; CHECK-NEXT: call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL]]) + +define void @test0() { + %call1 = notail call i8* @foo() [ "clang.arc.attachedcall"() ] + call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %call1) + ret void +} + +; CHECK-LABEL: define void @test1() { +; CHECK: %[[CALL:.*]] = notail call i8* @foo() [ "clang.arc.attachedcall"() ] +; CHECK-NEXT: call void asm sideeffect "mov\09fp, fp\09\09// marker for objc_retainAutoreleaseReturnValue", ""() +; CHECK-NEXT: call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* %[[CALL]]) + +define void @test1() { + %call1 = notail call i8* @foo() [ "clang.arc.attachedcall"() ] + call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* %call1) + ret void +} + +declare i8* @foo() +declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*) +declare i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8*) + +!llvm.module.flags = !{!0} + +!0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov\09fp, fp\09\09// marker for objc_retainAutoreleaseReturnValue"}