From 3381f8fbc62b309dae1d819e1c512f7843d2d52c Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Tue, 13 Aug 2019 01:23:06 +0000 Subject: [PATCH] Do not call replaceAllUsesWith to upgrade calls to ARC runtime functions to intrinsic calls This fixes a bug in r368311. It turns out that the ARC runtime functions in the IR can have pointer parameter types that are not i8* or i8**. Instead of RAUWing normal functions with intrinsics, manually bitcast the arguments before passing them to the intrinsic functions and bitcast the return value back to the type of the original call instruction. This recommits r368634, which was reverted in r368637. The loop in the patch was iterating over uses of a function and deleting function calls inside it, which caused bots to crash. rdar://problem/54125406 Differential Revision: https://reviews.llvm.org/D66047 llvm-svn: 368646 --- lib/IR/AutoUpgrade.cpp | 41 +++++++++++++-- test/Bitcode/upgrade-arc-runtime-calls.bc | Bin 2896 -> 3040 bytes test/Bitcode/upgrade-arc-runtime-calls.ll | 60 +++++++++++++++++++--- test/Bitcode/upgrade-mrr-runtime-calls.bc | Bin 2720 -> 2880 bytes 4 files changed, 92 insertions(+), 9 deletions(-) diff --git a/lib/IR/AutoUpgrade.cpp b/lib/IR/AutoUpgrade.cpp index 6d1305393e1..e8443aeb715 100644 --- a/lib/IR/AutoUpgrade.cpp +++ b/lib/IR/AutoUpgrade.cpp @@ -3855,6 +3855,8 @@ bool llvm::UpgradeRetainReleaseMarker(Module &M) { } void llvm::UpgradeARCRuntimeCalls(Module &M) { + // This lambda converts normal function calls to ARC runtime functions to + // intrinsic calls. auto UpgradeToIntrinsic = [&](const char *OldFunc, llvm::Intrinsic::ID IntrinsicFunc) { Function *Fn = M.getFunction(OldFunc); @@ -3863,11 +3865,44 @@ void llvm::UpgradeARCRuntimeCalls(Module &M) { return; Function *NewFn = llvm::Intrinsic::getDeclaration(&M, IntrinsicFunc); - Fn->replaceAllUsesWith(NewFn); - Fn->eraseFromParent(); + + for (auto I = Fn->user_begin(), E = Fn->user_end(); I != E;) { + CallInst *CI = dyn_cast(*I++); + if (!CI || CI->getCalledFunction() != Fn) + continue; + + IRBuilder<> Builder(CI->getParent(), CI->getIterator()); + FunctionType *NewFuncTy = NewFn->getFunctionType(); + SmallVector Args; + + for (unsigned I = 0, E = CI->getNumArgOperands(); I != E; ++I) { + Value *Arg = CI->getArgOperand(I); + // Bitcast argument to the parameter type of the new function if it's + // not a variadic argument. + if (I < NewFuncTy->getNumParams()) + Arg = Builder.CreateBitCast(Arg, NewFuncTy->getParamType(I)); + Args.push_back(Arg); + } + + // Create a call instruction that calls the new function. + CallInst *NewCall = Builder.CreateCall(NewFuncTy, NewFn, Args); + NewCall->setTailCallKind(cast(CI)->getTailCallKind()); + NewCall->setName(CI->getName()); + + // Bitcast the return value back to the type of the old call. + Value *NewRetVal = Builder.CreateBitCast(NewCall, CI->getType()); + + if (!CI->use_empty()) + CI->replaceAllUsesWith(NewRetVal); + CI->eraseFromParent(); + } + + if (Fn->use_empty()) + Fn->eraseFromParent(); }; - // Unconditionally convert "clang.arc.use" to "llvm.objc.clang.arc.use". + // Unconditionally convert a call to "clang.arc.use" to a call to + // "llvm.objc.clang.arc.use". UpgradeToIntrinsic("clang.arc.use", llvm::Intrinsic::objc_clang_arc_use); // Return if the bitcode doesn't have the arm64 retainAutoreleasedReturnValue diff --git a/test/Bitcode/upgrade-arc-runtime-calls.bc b/test/Bitcode/upgrade-arc-runtime-calls.bc index 5fedabb890485913715d284a7d392d635a49ccf4..35c78004a951a5ff5e7c9fc7d5d33a5ad6615f67 100644 GIT binary patch delta 1424 zcmca0_CTEX-T`rL1|Sdt;u8~jwHehXT58lwxH36S;&wVBU?gI}A>qwsl+@u-a70nr zMX0?;z;O~&$_b8*A_~rK9SSEIjgnd+jbt!O65wTm%Ki`mK?8ZsO+ zm<*a67$*Mps|VQ(1riJlEIca}G#Hs+A`A=-4T_=?G28+O5s;d94A~7xA_@r}U%8^0 zSdfGmowZ6c}?h+~vNMbLVtQHzpz^nt>&%glm-%_{44`^m(O+E1dEh${J?mWnh z8qKR_EV5Go83F|y6K*I3l(+&7%i#`3!4?GxmDGS92@yw*rh`Fj$2u56LMAB&NdYX19BiDDGRkaj z0!;}YCLBzJsB$!6JKoU}rJ;?PkwF0%YU&#TdRt8nZ?kABV)TCO!m}Z#TUle8vQBp^OG771V=qh7M3&~M zEG;uRM03@hLZ!O`ZcbAQ*}jrPG*8=Us&se2%WFy@$-8%Qi011%U6prmGzo5zJQHwr z%Tb=t*x5kFwzE8;wbEw;uI{+X6FPS`kg@A7Pv~Cha{*UvfcAqyp#lQ~GmvG(C!o~A zup7h#gIFF0hW$X65c8A|n-e&uGP%Xar&mkTz2C;8aLy zWtptBqy|JS(4GOUJz(J90(XJX;NV3fb$s`nA$=T z3KSiXM1X2`G5atwA%q$l6t8OMun8bU7+82#URjvffFz=j;8DsyftdwKh|zh~wgSkEdXr3_3!45~k76l23M1vj)5l4OgD%U$7P{VcV&p00u8PQn(Px62o*~Jivfksga%6S0i#L+ z7&_ks8Q6qa#DqB1ltQ-ja)jopIHg8+2HZ_k3fVE2BQ#ILX=!vHe9b7NG*{0#^!7KSkgvH0&Y_RRj6+KEjGRMXn;D0E%`Grd|HfuCt7OMOSm#QP2zSsB48w9!6D(zWt7z6QE)_2 z*+r;%xHP6Y-ASs-ns)WNBc z(#j&Nyrcz4JZ@PyK}5)>r-!@ch>yUGgB%BB7@Zh7i&+vJSVWI9v?OZqSam6IOcAkU z6v)tQbnumINL;X-)81gi1CzE?CWTNFgAMxnY}<08^={!i(9;92znl zGnfpT92h44^{WTj3@TJsOaNfM%Jh z2cVg?DzH&c0Z9?ie`mWUqxnxOW)?4+_udt;aUz^v4-WIJG%qy!TFpYy?c28z&Fw*F z&rM}O4NtGyd3FfaFY|_pBBgR=}(S+7C?22_8$`5Qbw80 zO`s{^!-Rv05LJ#QY{xqqL8^R`97L4aT!FFyoxqF+)-Y8<1*)N=aT3&6A-2;{n-VsF zh3cCenb^+2#4bS1Wnw!A6Z+62Apvp_D;vK_QUb^!H@`bTtlCg}5vnTA1Y%{UdN0`d zj*omHN+wiXfhjSCD4ClMRq{L*b)Tz|Cn&A=_7Sh~{ZKO_lBrczI1JBzgBv4$*vlr>pV~jwZn^l4k<0 zZaK;m8ao@v*mjmDv{w3Tz||dBc|zyT1~PWtcAp2`6@B}GR7 delta 1251 zcmX>gwm_8c-T`rL1|Sdt;tsBfd|HfmCR%FLi?}j5P2zT1B48w9!6D(zWfaumQE)_2 z*+r!Ffg@+ zBort*Ac+9g>|*v|WI_lvG$>xx&S4Wkh%m75th}-?u>na$A;F`Re*!ZLk`SZws>$^k z3JOR<91=knU6dIa5kd|JCTQtsaiY1Y?4p4KLJb4ZO;WsU=x)l^L3dNhVWS2lZ9q5G zv&ttXFreAzcH{s9n*ZiFpqu5!x(LlIQ*AyLH2)Q3>L?&t2=w00(~=Sn2%&nA+gHsz ziDsYZtq^qkejP)1yQ%ebMpXN>zE&Ec+4r=6GFouHYB|;b4=PWfQ-JX|RjQQ{J#Ipk znAy;R{k;ks4@xw9&s!8cksVA~f)fFe8 zQXbQxQU_I)*?`GDA)(+@M?J*eJ4R4Tj>FWLLbN*+_8KQ8fb73;%>t?|_dL|Kw~QgW z+Sq!*u5MiD3RRME38usZqGYBUM2X|FP^h~yL`{JP*8@%V2@8aZC4j|%LT5q)rTBnR zB>@bbZ-NYLLM&oJ9BN7-TY5P{b5)#DqdNocrYVK&n9C8Gr{T0Tx+~!AGNq6`YdJ#e z8CWm+2&U;chhDyB6jGY2=Nx+bn^DNuTm$FO$704IrFllqp|8!1L%!yjIEQ|oEXW}$ z_79$RV|f@Db^+}aVxF>Rvkk{oCL==wJs>PD%Tq`*PBcleurN=wFio+rG)poxGdD;| bOfgI|Hcv9QNHt0{HZe0ax3rin&aDUlmQMx(