From 362b4066f0c6178d639db41517549e13b3610036 Mon Sep 17 00:00:00 2001 From: Ahmed Bougacha Date: Fri, 20 May 2022 15:26:31 -0700 Subject: [PATCH] [ObjCARC] Drop nullary clang.arc.attachedcall bundles in autoupgrade. In certain use-cases, these can be emitted by old compilers, but the operand is now always required. These are only used for optimizations, so it's safe to drop them if they happen to have the now-invalid format. The semantically-required call is already a separate instruction. Differential Revision: https://reviews.llvm.org/D123811 --- llvm/include/llvm/IR/AutoUpgrade.h | 7 +++++++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 12 +++++++++++ llvm/lib/IR/AutoUpgrade.cpp | 12 +++++++++++ .../Bitcode/upgrade-arc-attachedcall-bundle.ll | 23 +++++++++++++++++++++ .../Bitcode/upgrade-arc-attachedcall-bundle.ll.bc | Bin 0 -> 1364 bytes 5 files changed, 54 insertions(+) create mode 100644 llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll create mode 100644 llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll.bc diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h index bcece46..96d20a7 100644 --- a/llvm/include/llvm/IR/AutoUpgrade.h +++ b/llvm/include/llvm/IR/AutoUpgrade.h @@ -14,6 +14,7 @@ #define LLVM_IR_AUTOUPGRADE_H #include "llvm/ADT/StringRef.h" +#include namespace llvm { class AttrBuilder; @@ -28,6 +29,9 @@ namespace llvm { class Type; class Value; + template class OperandBundleDefT; + using OperandBundleDef = OperandBundleDefT; + /// This is a more granular function that simply checks an intrinsic function /// for upgrading, and returns true if it requires upgrading. It may return /// null in NewFn if the all calls to the original intrinsic function @@ -99,6 +103,9 @@ namespace llvm { /// Upgrade attributes that changed format or kind. void UpgradeAttributes(AttrBuilder &B); + /// Upgrade operand bundles (without knowing about their user instruction). + void UpgradeOperandBundles(std::vector &OperandBundles); + } // End llvm namespace #endif diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 9d49a96..cc58929 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -5138,6 +5138,10 @@ Error BitcodeReader::parseFunctionBody(Function *F) { } } + // Upgrade the bundles if needed. + if (!OperandBundles.empty()) + UpgradeOperandBundles(OperandBundles); + I = InvokeInst::Create(FTy, Callee, NormalBB, UnwindBB, Ops, OperandBundles); ResTypeID = getContainedTypeID(FTyID); @@ -5235,6 +5239,10 @@ Error BitcodeReader::parseFunctionBody(Function *F) { } } + // Upgrade the bundles if needed. + if (!OperandBundles.empty()) + UpgradeOperandBundles(OperandBundles); + I = CallBrInst::Create(FTy, Callee, DefaultDest, IndirectDests, Args, OperandBundles); ResTypeID = getContainedTypeID(FTyID); @@ -5846,6 +5854,10 @@ Error BitcodeReader::parseFunctionBody(Function *F) { } } + // Upgrade the bundles if needed. + if (!OperandBundles.empty()) + UpgradeOperandBundles(OperandBundles); + I = CallInst::Create(FTy, Callee, Args, OperandBundles); ResTypeID = getContainedTypeID(FTyID); OperandBundles.clear(); diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp index fbd0877..8028371 100644 --- a/llvm/lib/IR/AutoUpgrade.cpp +++ b/llvm/lib/IR/AutoUpgrade.cpp @@ -4675,3 +4675,15 @@ void llvm::UpgradeAttributes(AttrBuilder &B) { B.addAttribute(Attribute::NullPointerIsValid); } } + +void llvm::UpgradeOperandBundles(std::vector &Bundles) { + + // clang.arc.attachedcall bundles are now required to have an operand. + // If they don't, it's okay to drop them entirely: when there is an operand, + // the "attachedcall" is meaningful and required, but without an operand, + // it's just a marker NOP. Dropping it merely prevents an optimization. + erase_if(Bundles, [&](OperandBundleDef &OBD) { + return OBD.getTag() == "clang.arc.attachedcall" && + OBD.inputs().empty(); + }); +} diff --git a/llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll b/llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll new file mode 100644 index 0000000..7b012ab --- /dev/null +++ b/llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll @@ -0,0 +1,23 @@ +; Test that nullary clang.arc.attachedcall operand bundles are "upgraded". + +; RUN: llvm-dis %s.bc -o - | FileCheck %s +; RUN: verify-uselistorder %s.bc + +define i8* @invalid() { +; CHECK-LABEL: define i8* @invalid() { +; CHECK-NEXT: %tmp0 = call i8* @foo(){{$}} +; CHECK-NEXT: ret i8* %tmp0 + %tmp0 = call i8* @foo() [ "clang.arc.attachedcall"() ] + ret i8* %tmp0 +} + +define i8* @valid() { +; CHECK-LABEL: define i8* @valid() { +; CHECK-NEXT: %tmp0 = call i8* @foo() [ "clang.arc.attachedcall"(i8* (i8*)* @llvm.objc.retainAutoreleasedReturnValue) ] +; CHECK-NEXT: ret i8* %tmp0 + %tmp0 = call i8* @foo() [ "clang.arc.attachedcall"(i8* (i8*)* @llvm.objc.retainAutoreleasedReturnValue) ] + ret i8* %tmp0 +} + +declare i8* @foo() +declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*) diff --git a/llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll.bc b/llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll.bc new file mode 100644 index 0000000000000000000000000000000000000000..bd47cfc1bebcb4a1e8ecccb99d8a25b248dc5f48 GIT binary patch literal 1364 zcmY*ZacC1~6#uT5=1v>GyJ^wX-o-mxS=Np2l2lEWWHdP^k##O^V|3tHE+(0ES$j>> zHm%!8nz*Z&L6pKW=)i*i_+JP8Ls%JYuXC9Mak33zp(Jb6PNECb4PnCeU39`8-1~Ts z_r3Rfzwf>Cic4#ITLEYW00a#}=WFkL6Z!1#h1c5)od+;$GvEyXbYTFrH1VJb#cW3( zk?J_>Dd-OAX{+NLmND#UCfiK9Gr=VNN|X0oi#J1CTkWLr)y-D#v`=2;I}U7Bvb5)1 zQ>Z|G)yjznr@8Mjt2blpGI!-HAI%><%dHkr-qRcas2s5FCO2`xgMfd&Gf(q-kZOC@ zaOy%X;0+z+Izv<7i;iBhkM2Zyc9KR695DibqkHYs<$sV6y$I+b~E^>yO2*GL5`X1qOVhty*9I$r5I`nD;bVed4kw`JYDgI=Ydr;akFdQQCZGD**Hv zt;Xl*vyn+=wZcSNG554+&!|~%l9)&kBl%&&EkNC+;e#Z!8N8;IW%lYE6A_vlVd?fF z?kO>NfHx2MV5(486Qh1GN-f^pMkX;tO(SgK&{td-4AW_D!axrH+XhM zGDU>b&c(Fzn&?=-U5jZ~26qH1)&x9Qm}9DQOtGO$5_5%TR#WU1N&jmF59Z`@a)<$`w3>!0P}2J(!X^oZ7-_MdC`psd9>oHiq2_ds@ZQhqfV7^goh+1%|tAp zI=W87+7i6!hxdh!A>t(NR=gmzEs`74_&U`WA*s z0-+j41okU!zg&j30Cl&luXEH*fx5|2fBE4c4c7&T%==~iZ)I5Z!-qo0uQiETNwGI7 zJ8C?8i)UAP7QNsGzbnFh>0A`umvF~S+EMIZM6ZvUokTc&%D7Aba1qTik7lf8bIgq1 zj+pG1^C(_LEIF>JQ@5DQb6Yk%+GwG)k2%@ivmSuA{fKI+su51z_QMDB$rLeRZcIW# zL0xD(^vkoVeF1Y`NV{j$rVr3iF{KJq;+U$W>5Y_WMPjbaF@?wAu^-U-=n)p8=(>=0 z&Y}Up-RILTGz>Xa+XaaOU=upG>6S~LK-NeDaw^3|X{ZB@r8?N#R}{5nMIvzkY(b|M zn+oVU^g1vV;K=q2j{Pr|w>-Yv@+o+_yu;|Omc|O{-5D^`WUN1jECAj`KagGM+=|cq zuGs^FV^VY=)KHP3p=fmM!|tKriDTWda9kP~^o+)bV&Q058VQHq2**cbgKtaG(Xgeb zTj&