[Bitcode] Fix bitcode compatibility issue with clang.arc.use intrinsic

[steven_wu]

Summary

In r349534, objc arc implementation is switched to use intrinsics and at
the same time, clang.arc.use is renamed to llvm.objc.clang.arc.use to
make the naming more consistent. The side-effect of that is llvm no
longer recognize it as intrinsics and codegen external references to
it instead.

Rather than upgrade the old intrinsics name to the new one and wait for
the arc-contract pass to remove it, simply remove it in the bitcode
upgrader.

rdar://problem/48607063

[pete]

LGTM.

[dexonsmith]

LGTM too with some nitpicks (forgot to hit "submit" earlier this afternoon).

lib/IR/AutoUpgrade.cpp

793–794 ↗

(On Diff #189787)

Are we going to grow more intrinsics that don't start with llvm.? I feel like having clang. was a mistake to do, and I wonder if the logic is even worth splitting out like this.

If you still think it's worth splitting, I suggest renaming UpgradeIntrinsicFunction1 while you're in there to something meaningful (maybe in an NFC commit ahead of time), like upgradeLLVMIntrinsicFunction. Then I'd drop the unused parameter from the new one, and call it upgradeClangARCIntrinsicFunction(). This might avoid encouraging new intrinsic names that don't start with llvm..

1585 ↗

(On Diff #189787)

I think this deserves a comment for why this is correct or reasonable. Maybe something like: "clang.arc.use is an old name for llvm.arc.clang.arc.use, but we might as well drop it entirely because the optimizer now only recognizes intrinsics for ARC runtime calls."

test/Bitcode/upgrade-clang-arc-use.ll

3 ↗

(On Diff #189787)

Can you document what released version of LLVM was used to generate this bitcode file?

[dexonsmith]

lib/IR/AutoUpgrade.cpp

793–794 ↗

(On Diff #189787)

Then I'd drop the unused parameter from the new one

Well, I guess it's not unused, but I'd simplify the logic so it doesn't look like things should be added.

llvm/trunk/lib/IR/AutoUpgrade.cpp

static bool UpgradeX86IntrinsicFunction(Function *F, StringRef Name,
   }
 
   return false;
 }
 
 static bool UpgradeIntrinsicFunction1(Function *F, Function *&NewFn) {
   assert(F && "Illegal to upgrade a non-existent Function.");
 
+  // Upgrade intrinsics "clang.arc.use" which doesn't start with "llvm.".
+  if (F->getName() == "clang.arc.use") {
+    NewFn = nullptr;
+    return true;
+  }
+
   // Quickly eliminate it, if it's not a candidate.
   StringRef Name = F->getName();
   if (Name.size() <= 8 || !Name.startswith("llvm."))
     return false;
   Name = Name.substr(5); // Strip off "llvm."
 
   switch (Name[0]) {
   default: break;
void llvm::UpgradeIntrinsicCall(CallInst *CI, Function *NewFn) {
   Builder.SetInsertPoint(CI->getParent(), CI->getIterator());
 
   assert(F && "Intrinsic call is not direct?");
 
   if (!NewFn) {
     // Get the Function's name.
     StringRef Name = F->getName();
 
+    // clang.arc.use is an old name for llvm.arc.clang.arc.use. It is dropped
+    // from upgrader because the optimizer now only recognizes intrinsics for
+    // ARC runtime calls.
+    if (Name == "clang.arc.use") {
+      CI->eraseFromParent();
+      return;
+    }
+
     assert(Name.startswith("llvm.") && "Intrinsic doesn't start with 'llvm.'");
     Name = Name.substr(5);
 
     bool IsX86 = Name.startswith("x86.");
     if (IsX86)
       Name = Name.substr(4);
     bool IsNVVM = Name.startswith("nvvm.");
     if (IsNVVM)

llvm/trunk/test/Bitcode/upgrade-clang-arc-use.ll

+; Test upgrade of clang.arc.use by removing it.
+; Bitcode input generated from llvm 6.0
+
+; RUN: llvm-dis %s.bc -o - | FileCheck %s
+
+%0 = type opaque
+define void @foo() {
+  %1 = tail call %0* @foo0()
+; CHECK-NOT: clang.arc.use
+  call void (...) @clang.arc.use(%0* %1)
+  ret void
+}
+declare %0* @foo0()
+declare void @clang.arc.use(...)