[AutoUpgrader] Make ArcRuntime Autoupgrader more conservative

Summary:
This is a tweak to r368311 and r368646 which auto upgrades the calls to
objc runtime functions to objc runtime intrinsics, in order to make sure
that the auto upgrader does not trigger with up-to-date bitcode.

It is possible for bitcode that is up-to-date to contain direct calls to
objc runtime function and those are not inserted by compiler as part of
ARC and they should not be upgraded. Now auto upgrader only triggers as
when the old style of ARC marker is used so it is guaranteed that it
won't trigger on update-to-date bitcode.

This also means it won't do this upgrade for bitcode from llvm-8 and
llvm-9, which preserves the behavior of those releases. Ideally they
should be upgraded as well but it is more important to make sure
AutoUpgrader will not trigger on up-to-date bitcode.

Reviewers: ahatanak, rjmccall, dexonsmith, pete

Reviewed By: dexonsmith

Subscribers: hiraditya, jkorous, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D66153

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368730 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/AutoUpgrade.h b/include/llvm/IR/AutoUpgrade.h
index 8613a1545ea72e5dd48411993f15c5a0161e3fb8..99ab68977a4f8d4a797dd82b7d8d422a3c696dbb 100644 (file)
--- a/include/llvm/IR/AutoUpgrade.h
+++ b/include/llvm/IR/AutoUpgrade.h
@@ -54,13 +54,9 @@ namespace llvm {
   /// module is modified.
   bool UpgradeModuleFlags(Module &M);
 
-  /// This checks for objc retain release marker which should be upgraded. It
-  /// returns true if module is modified.
-  bool UpgradeRetainReleaseMarker(Module &M);
-
-  /// Convert calls to ARC runtime functions to intrinsic calls if the bitcode
-  /// has the arm64 retainAutoreleasedReturnValue marker.
-  void UpgradeARCRuntimeCalls(Module &M);
+  /// Convert calls to ARC runtime functions to intrinsic calls and upgrade the
+  /// old retain release marker to new module flag format.
+  void UpgradeARCRuntime(Module &M);
 
   void UpgradeSectionAttributes(Module &M);
 

diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index cba9385318c2c2a20882c298276aa216438e9c62..997b556f0f99f838ebb56627dcf6a74b577411d8 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -5312,8 +5312,7 @@ Error BitcodeReader::materializeModule() {
 
   UpgradeModuleFlags(*TheModule);
 
-  UpgradeRetainReleaseMarker(*TheModule);
-  UpgradeARCRuntimeCalls(*TheModule);
+  UpgradeARCRuntime(*TheModule);
 
   return Error::success();
 }

diff --git a/lib/IR/AutoUpgrade.cpp b/lib/IR/AutoUpgrade.cpp
index e8443aeb715276e24f06f8e7116fe12f93b2500d..e9d49d8a2b5cb6882f767a95cf2909fc97c3dcbb 100644 (file)
--- a/lib/IR/AutoUpgrade.cpp
+++ b/lib/IR/AutoUpgrade.cpp
@@ -3830,7 +3830,9 @@ bool llvm::UpgradeDebugInfo(Module &M) {
   return Modified;
 }
 
-bool llvm::UpgradeRetainReleaseMarker(Module &M) {
+/// 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";
   NamedMDNode *ModRetainReleaseMarker = M.getNamedMetadata(MarkerKey);
@@ -3854,7 +3856,7 @@ bool llvm::UpgradeRetainReleaseMarker(Module &M) {
   return Changed;
 }
 
-void llvm::UpgradeARCRuntimeCalls(Module &M) {
+void llvm::UpgradeARCRuntime(Module &M) {
   // This lambda converts normal function calls to ARC runtime functions to
   // intrinsic calls.
   auto UpgradeToIntrinsic = [&](const char *OldFunc,
@@ -3905,9 +3907,10 @@ void llvm::UpgradeARCRuntimeCalls(Module &M) {
   // "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
-  // marker. We don't know for sure that it was compiled with ARC in that case.
-  if (!M.getModuleFlag("clang.arc.retainAutoreleasedReturnValueMarker"))
+  // Upgrade the retain release marker. If there is no need to upgrade
+  // the marker, that means either the module is already new enough to contain
+  // new intrinsics or it is not ARC. There is no need to upgrade runtime call.
+  if (!UpgradeRetainReleaseMarker(M))
     return;
 
   std::pair<const char *, llvm::Intrinsic::ID> RuntimeFuncs[] = {

diff --git a/test/Bitcode/upgrade-arc-runtime-calls-new.bc b/test/Bitcode/upgrade-arc-runtime-calls-new.bc
new file mode 100644 (file)
index 0000000..68e350a
Binary files /dev/null and b/test/Bitcode/upgrade-arc-runtime-calls-new.bc differ

diff --git a/test/Bitcode/upgrade-arc-runtime-calls.ll b/test/Bitcode/upgrade-arc-runtime-calls.ll
index dbccff2a5b0d15cfb338f58be0af1ca5192ba637..d42c776ddc51819196058fe2e7b9c54b83a2bc32 100644 (file)
--- a/test/Bitcode/upgrade-arc-runtime-calls.ll
+++ b/test/Bitcode/upgrade-arc-runtime-calls.ll
@@ -3,10 +3,12 @@
 
 ; upgrade-arc-runtime-calls.bc and upgrade-mrr-runtime-calls.bc are identical
 ; except that the former has the arm64 retainAutoreleasedReturnValueMarker
-; metadata.
+; metadata. upgrade-arc-runtime-calls-new.bc has the new module flag format of
+; marker, it should not be upgraded.
 
 ; RUN: llvm-dis < %S/upgrade-arc-runtime-calls.bc | FileCheck -check-prefixes=ARC %s
-; RUN: llvm-dis < %S/upgrade-mrr-runtime-calls.bc | FileCheck -check-prefixes=MRR %s
+; RUN: llvm-dis < %S/upgrade-mrr-runtime-calls.bc | FileCheck -check-prefixes=NOUPGRADE %s
+; RUN: llvm-dis < %S/upgrade-arc-runtime-calls-new.bc | FileCheck -check-prefixes=NOUPGRADE %s
 
 define void @testRuntimeCalls(i8* %a, i8** %b, i8** %c, i32* %d, i32** %e) personality i32 (...)* @__gxx_personality_v0 {
 entry:
@@ -89,35 +91,35 @@ unwindBlock:
 // ARC-NEXT: tail call void @llvm.objc.arc.annotation.bottomup.bbend(i8** %[[B]], i8** %[[C]])
 // ARC-NEXT: invoke void @objc_autoreleasePoolPop(i8* %[[A]])
 
-// MRR: define void @testRuntimeCalls(i8* %[[A:.*]], i8** %[[B:.*]], i8** %[[C:.*]], i32* %[[D:.*]], i32** %[[E:.*]]) personality
-// MRR: %[[V0:.*]] = tail call i8* @objc_autorelease(i8* %[[A]])
-// MRR-NEXT: tail call void @objc_autoreleasePoolPop(i8* %[[A]])
-// MRR-NEXT: %[[V1:.*]] = tail call i8* @objc_autoreleasePoolPush()
-// MRR-NEXT: %[[V2:.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* %[[A]])
-// MRR-NEXT: tail call void @objc_copyWeak(i8** %[[B]], i8** %[[C]])
-// MRR-NEXT: tail call void @objc_destroyWeak(i8** %[[B]])
-// MRR-NEXT: %[[V3:.*]] = tail call i32* @objc_initWeak(i32** %[[E]], i32* %[[D]])
-// MRR-NEXT: %[[V4:.*]] = tail call i8* @objc_loadWeak(i8** %[[B]])
-// MRR-NEXT: %[[V5:.*]] = tail call i8* @objc_loadWeakRetained(i8** %[[B]])
-// MRR-NEXT: tail call void @objc_moveWeak(i8** %[[B]], i8** %[[C]])
-// MRR-NEXT: tail call void @objc_release(i8* %[[A]])
-// MRR-NEXT: %[[V6:.*]] = tail call i8* @objc_retain(i8* %[[A]])
-// MRR-NEXT: %[[V7:.*]] = tail call i8* @objc_retainAutorelease(i8* %[[A]])
-// MRR-NEXT: %[[V8:.*]] = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %[[A]])
-// MRR-NEXT: %[[V9:.*]] = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %[[A]])
-// MRR-NEXT: %[[V10:.*]] = tail call i8* @objc_retainBlock(i8* %[[A]])
-// MRR-NEXT: tail call void @objc_storeStrong(i8** %[[B]], i8* %[[A]])
-// MRR-NEXT: %[[V11:.*]] = tail call i8* @objc_storeWeak(i8** %[[B]], i8* %[[A]])
-// MRR-NEXT: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[A]])
-// MRR-NEXT: %[[V12:.*]] = tail call i8* @objc_unsafeClaimAutoreleasedReturnValue(i8* %[[A]])
-// MRR-NEXT: %[[V13:.*]] = tail call i8* @objc_retainedObject(i8* %[[A]])
-// MRR-NEXT: %[[V14:.*]] = tail call i8* @objc_unretainedObject(i8* %[[A]])
-// MRR-NEXT: %[[V15:.*]] = tail call i8* @objc_unretainedPointer(i8* %[[A]])
-// MRR-NEXT: %[[V16:.*]] = tail call i8* @objc_retain.autorelease(i8* %[[A]])
-// MRR-NEXT: %[[V17:.*]] = tail call i32 @objc_sync.enter(i8* %[[A]])
-// MRR-NEXT: %[[V18:.*]] = tail call i32 @objc_sync.exit(i8* %[[A]])
-// MRR-NEXT: tail call void @objc_arc_annotation_topdown_bbstart(i8** %[[B]], i8** %[[C]])
-// MRR-NEXT: tail call void @objc_arc_annotation_topdown_bbend(i8** %[[B]], i8** %[[C]])
-// MRR-NEXT: tail call void @objc_arc_annotation_bottomup_bbstart(i8** %[[B]], i8** %[[C]])
-// MRR-NEXT: tail call void @objc_arc_annotation_bottomup_bbend(i8** %[[B]], i8** %[[C]])
-// MRR-NEXT: invoke void @objc_autoreleasePoolPop(i8* %[[A]])
+// NOUPGRADE: define void @testRuntimeCalls(i8* %[[A:.*]], i8** %[[B:.*]], i8** %[[C:.*]], i32* %[[D:.*]], i32** %[[E:.*]]) personality
+// NOUPGRADE: %[[V0:.*]] = tail call i8* @objc_autorelease(i8* %[[A]])
+// NOUPGRADE-NEXT: tail call void @objc_autoreleasePoolPop(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V1:.*]] = tail call i8* @objc_autoreleasePoolPush()
+// NOUPGRADE-NEXT: %[[V2:.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* %[[A]])
+// NOUPGRADE-NEXT: tail call void @objc_copyWeak(i8** %[[B]], i8** %[[C]])
+// NOUPGRADE-NEXT: tail call void @objc_destroyWeak(i8** %[[B]])
+// NOUPGRADE-NEXT: %[[V3:.*]] = tail call i32* @objc_initWeak(i32** %[[E]], i32* %[[D]])
+// NOUPGRADE-NEXT: %[[V4:.*]] = tail call i8* @objc_loadWeak(i8** %[[B]])
+// NOUPGRADE-NEXT: %[[V5:.*]] = tail call i8* @objc_loadWeakRetained(i8** %[[B]])
+// NOUPGRADE-NEXT: tail call void @objc_moveWeak(i8** %[[B]], i8** %[[C]])
+// NOUPGRADE-NEXT: tail call void @objc_release(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V6:.*]] = tail call i8* @objc_retain(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V7:.*]] = tail call i8* @objc_retainAutorelease(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V8:.*]] = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V9:.*]] = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V10:.*]] = tail call i8* @objc_retainBlock(i8* %[[A]])
+// NOUPGRADE-NEXT: tail call void @objc_storeStrong(i8** %[[B]], i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V11:.*]] = tail call i8* @objc_storeWeak(i8** %[[B]], i8* %[[A]])
+// NOUPGRADE-NEXT: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V12:.*]] = tail call i8* @objc_unsafeClaimAutoreleasedReturnValue(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V13:.*]] = tail call i8* @objc_retainedObject(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V14:.*]] = tail call i8* @objc_unretainedObject(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V15:.*]] = tail call i8* @objc_unretainedPointer(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V16:.*]] = tail call i8* @objc_retain.autorelease(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V17:.*]] = tail call i32 @objc_sync.enter(i8* %[[A]])
+// NOUPGRADE-NEXT: %[[V18:.*]] = tail call i32 @objc_sync.exit(i8* %[[A]])
+// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_topdown_bbstart(i8** %[[B]], i8** %[[C]])
+// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_topdown_bbend(i8** %[[B]], i8** %[[C]])
+// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_bottomup_bbstart(i8** %[[B]], i8** %[[C]])
+// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_bottomup_bbend(i8** %[[B]], i8** %[[C]])
+// NOUPGRADE-NEXT: invoke void @objc_autoreleasePoolPop(i8* %[[A]])