ObjCARC: teach the cloner about funclets

[compnerd]

Summary

In the case that the CallInst that is being moved has an associated
operand bundle which is a funclet, the move will construct an invalid
instruction. The new site will have a different token and needs to be
reassociated with the new instruction.

Unfortunately, there is no way to alter the bundle after the
construction of the instruction. Replace the call instruction cloning
with a custom helper to clone the instruction and reassociate the
funclet token.

[majnemer]

lib/Transforms/ObjCARC/ObjCARCOpts.cpp

702

What if the cleanuppad was introduced in a block which branched to this one?

[compnerd]

Use the BB colorizer to detect the token. Fortunately, there is no BB removal/splitting happening here, so there is no state to maintain.

[majnemer]

LGTM

lib/Transforms/ObjCARC/ObjCARCOpts.cpp

692

dyn_cast -> cast

Actually, why not just take a CallInst &?

698

Start the comment with an uppercase and end with a period.

[compnerd]

SVN r327336. Addressed comments in SVN r327351, because I forgot to incorporate them in the first try.

lib/Transforms/ObjCARC/ObjCARCOpts.cpp

 #include "PtrState.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
 #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/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
void ObjCARCOpt::OptimizeAutoreleaseRVCall(Function &F,
   Constant *NewDecl = EP.get(ARCRuntimeEntryPointKind::Autorelease);
   AutoreleaseRVCI->setCalledFunction(NewDecl);
   AutoreleaseRVCI->setTailCall(false); // Never tail call objc_autorelease.
   Class = ARCInstKind::Autorelease;
 
   DEBUG(dbgs() << "New: " << *AutoreleaseRV << "\n");
 }
 
+namespace {
+Instruction *
+CloneCallInstForBB(Instruction &I, BasicBlock &BB,
+                   DenseMap<BasicBlock *, ColorVector> &BlockColors) {
+  auto *CI = dyn_cast<CallInst>(&I);
+  assert(CI && "CloneCallInst must receive a CallInst");
+
+  SmallVector<OperandBundleDef, 1> OpBundles;
+  for (unsigned I = 0, E = CI->getNumOperandBundles(); I != E; ++I) {
+    auto Bundle = CI->getOperandBundleAt(I);
+    // funclets will be reassociated in the future
+    if (Bundle.getTagID() == LLVMContext::OB_funclet)
+      continue;
+    OpBundles.emplace_back(Bundle);
+  }
+
+  if (!BlockColors.empty()) {
+    const ColorVector &CV = BlockColors.find(&BB)->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(CI, OpBundles);
+}
+}
+
 /// Visit each call, one at a time, and make simplifications without doing any
 /// additional analysis.
 void ObjCARCOpt::OptimizeIndividualCalls(Function &F) {
   DEBUG(dbgs() << "\n== ObjCARCOpt::OptimizeIndividualCalls ==\n");
   // Reset all the flags in preparation for recomputing them.
   UsedInThisFunction = 0;
 
+  DenseMap<BasicBlock *, ColorVector> BlockColors;
+  if (F.hasPersonalityFn() &&
+      isFuncletEHPersonality(classifyEHPersonality(F.getPersonalityFn())))
+    BlockColors = colorEHFunclets(F);
+
   // Visit all objc_* calls in F.
   for (inst_iterator I = inst_begin(&F), E = inst_end(&F); I != E; ) {
     Instruction *Inst = &*I++;
 
     ARCInstKind Class = GetBasicARCInstKind(Inst);
 
     DEBUG(dbgs() << "Visiting: Class: " << Class << "; " << *Inst << "\n");
 
do {
           ++NumPartialNoops;
           // Clone the call into each predecessor that has a non-null value.
           CallInst *CInst = cast<CallInst>(Inst);
           Type *ParamTy = CInst->getArgOperand(0)->getType();
           for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
             Value *Incoming =
               GetRCIdentityRoot(PN->getIncomingValue(i));
             if (!IsNullOrUndef(Incoming)) {
-              CallInst *Clone = cast<CallInst>(CInst->clone());
               Value *Op = PN->getIncomingValue(i);
               Instruction *InsertPos = &PN->getIncomingBlock(i)->back();
+              CallInst *Clone = cast<CallInst>(CloneCallInstForBB(
+                  *CInst, *InsertPos->getParent(), BlockColors));
               if (Op->getType() != ParamTy)
                 Op = new BitCastInst(Op, ParamTy, "", InsertPos);
               Clone->setArgOperand(0, Op);
               Clone->insertBefore(InsertPos);
 
               DEBUG(dbgs() << "Cloning "
                            << *CInst << "\n"
                            "And inserting clone at " << *InsertPos << "\n");

test/Transforms/ObjCARC/funclet.ll

+; RUN: opt -mtriple x86_64-unknown-windows-msvc -objc-arc -S -o - %s | FileCheck %s
+
+; bool g();
+; id h();
+;
+; void f() {
+;   id a = nullptr;
+;   if (g())
+;     a = h();
+;   id b = nullptr;
+;   g();
+; }
+
+declare zeroext i1 @"\01?g@@YA_NXZ"() local_unnamed_addr
+declare i8* @"\01?h@@YAPEAUobjc_object@@XZ"() local_unnamed_addr
+
+declare dllimport void @objc_release(i8*) local_unnamed_addr
+declare dllimport i8* @objc_retainAutoreleasedReturnValue(i8* returned) local_unnamed_addr
+
+declare i32 @__CxxFrameHandler3(...)
+
+define void @"\01?f@@YAXXZ"() local_unnamed_addr personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+  %call = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+          to label %invoke.cont unwind label %ehcleanup6
+
+invoke.cont:                                      ; preds = %entry
+  br i1 %call, label %if.then, label %if.end
+
+if.then:                                          ; preds = %invoke.cont
+  %call2 = invoke i8* @"\01?h@@YAPEAUobjc_object@@XZ"()
+          to label %invoke.cont1 unwind label %ehcleanup6
+
+invoke.cont1:                                     ; preds = %if.then
+  %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call2)
+  tail call void @objc_release(i8* null), !clang.imprecise_release !1
+  br label %if.end
+
+if.end:                                           ; preds = %invoke.cont1, %invoke.cont
+  %a.0 = phi i8* [ %call2, %invoke.cont1 ], [ null, %invoke.cont ]
+  %call4 = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+          to label %invoke.cont3 unwind label %ehcleanup
+
+invoke.cont3:                                     ; preds = %if.end
+  tail call void @objc_release(i8* null), !clang.imprecise_release !1
+  tail call void @objc_release(i8* %a.0), !clang.imprecise_release !1
+  ret void
+
+ehcleanup:                                        ; preds = %if.end
+  %1 = cleanuppad within none []
+  call void @objc_release(i8* null) [ "funclet"(token %1) ], !clang.imprecise_release !1
+  cleanupret from %1 unwind label %ehcleanup6
+
+ehcleanup6:                                       ; preds = %ehcleanup, %if.then, %entry
+  %a.1 = phi i8* [ %a.0, %ehcleanup ], [ null, %if.then ], [ null, %entry ]
+  %2 = cleanuppad within none []
+  call void @objc_release(i8* %a.1) [ "funclet"(token %2) ], !clang.imprecise_release !1
+  cleanupret from %2 unwind to caller
+}
+
+; CHECK-LABEL: ?f@@YAXXZ
+; CHECK: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %1) ]
+; CHECK-NOT: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %2) ]
+
+define void @"\01?i@@YAXXZ"() local_unnamed_addr personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+  %call = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+          to label %invoke.cont unwind label %ehcleanup6
+
+invoke.cont:                                      ; preds = %entry
+  br i1 %call, label %if.then, label %if.end
+
+if.then:                                          ; preds = %invoke.cont
+  %call2 = invoke i8* @"\01?h@@YAPEAUobjc_object@@XZ"()
+          to label %invoke.cont1 unwind label %ehcleanup6
+
+invoke.cont1:                                     ; preds = %if.then
+  %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call2)
+  tail call void @objc_release(i8* null), !clang.imprecise_release !1
+  br label %if.end
+
+if.end:                                           ; preds = %invoke.cont1, %invoke.cont
+  %a.0 = phi i8* [ %call2, %invoke.cont1 ], [ null, %invoke.cont ]
+  %call4 = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+          to label %invoke.cont3 unwind label %ehcleanup
+
+invoke.cont3:                                     ; preds = %if.end
+  tail call void @objc_release(i8* null), !clang.imprecise_release !1
+  tail call void @objc_release(i8* %a.0), !clang.imprecise_release !1
+  ret void
+
+ehcleanup:                                        ; preds = %if.end
+  %1 = cleanuppad within none []
+  call void @objc_release(i8* null) [ "funclet"(token %1) ], !clang.imprecise_release !1
+  br label %ehcleanup.1
+
+ehcleanup.1:
+  cleanupret from %1 unwind label %ehcleanup6
+
+ehcleanup6:                                       ; preds = %ehcleanup, %if.then, %entry
+  %a.1 = phi i8* [ %a.0, %ehcleanup.1 ], [ null, %if.then ], [ null, %entry ]
+  %2 = cleanuppad within none []
+  call void @objc_release(i8* %a.1) [ "funclet"(token %2) ], !clang.imprecise_release !1
+  cleanupret from %2 unwind to caller
+}
+
+; CHECK-LABEL: ?i@@YAXXZ
+; CHECK: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %1) ]
+; CHECK-NOT: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %2) ]
+
+!1 = !{}
+