AutoUpgrade: Fix assertion on invalid name mangling usage

[arsenm]

Summary

This was trying to auto-upgrade a read_register call with missing type
mangling. This first would break since getCalledFunction checks the
callee type is consistent, so this would assert there. After that,
the replacement code would die on the type mismatch. Be more
defensive and let the verifier code produce an error that the IR
is broken.

[nikic]

LG

[arsenm]

e989b8bb5fb36abac6e8f82809f06144dd762113

llvm/lib/IR/AutoUpgrade.cpp

if (Name == "mve.vctp64.old") {
     return Builder.CreateCall(Fn, Ops, CI->getName());
   }
   llvm_unreachable("Unknown function for ARM CallBase upgrade.");
 }
 
 /// Upgrade a call to an old intrinsic. All argument and return casting must be
 /// provided to seamlessly integrate with existing context.
 void llvm::UpgradeIntrinsicCall(CallBase *CI, Function *NewFn) {
-  Function *F = CI->getCalledFunction();
+  // Note dyn_cast to Function is not quite the same as getCalledFunction, which
+  // checks the callee's function type matches. It's likely we need to handle
+  // type changes here.
+  Function *F = dyn_cast<Function>(CI->getCalledOperand());
+  if (!F)
+    return;
+
   LLVMContext &C = CI->getContext();
   IRBuilder<> Builder(C);
   Builder.SetInsertPoint(CI->getParent(), CI->getIterator());
 
-  assert(F && "Intrinsic call is not direct?");
-
   if (!NewFn) {
     // Get the Function's name.
     StringRef Name = F->getName();
 
     assert(Name.startswith("llvm.") && "Intrinsic doesn't start with 'llvm.'");
     Name = Name.substr(5);
 
     bool IsX86 = Name.startswith("x86.");
if (CI->getFunctionType() == NewFn->getFunctionType()) {
       assert(
           (CI->getCalledFunction()->getName() != NewFn->getName()) &&
           "Unknown function for CallBase upgrade and isn't just a name change");
       CI->setCalledFunction(NewFn);
       return;
     }
 
     // This must be an upgrade from a named to a literal struct.
-    auto *OldST = cast<StructType>(CI->getType());
-    assert(OldST != NewFn->getReturnType() && "Return type must have changed");
+    if (auto *OldST = dyn_cast<StructType>(CI->getType())) {
+      assert(OldST != NewFn->getReturnType() &&
+             "Return type must have changed");
       assert(OldST->getNumElements() ==
                  cast<StructType>(NewFn->getReturnType())->getNumElements() &&
              "Must have same number of elements");
 
       SmallVector<Value *> Args(CI->args());
       Value *NewCI = Builder.CreateCall(NewFn, Args);
       Value *Res = PoisonValue::get(OldST);
       for (unsigned Idx = 0; Idx < OldST->getNumElements(); ++Idx) {
         Value *Elem = Builder.CreateExtractValue(NewCI, Idx);
         Res = Builder.CreateInsertValue(Res, Elem, Idx);
       }
       CI->replaceAllUsesWith(Res);
       CI->eraseFromParent();
       return;
+    }
+
+    // We're probably about to produce something invalid. Let the verifier catch
+    // it instead of dying here.
+    CI->setCalledOperand(
+        ConstantExpr::getPointerCast(NewFn, CI->getCalledOperand()->getType()));
+    return;
   };
   CallInst *NewCall = nullptr;
   switch (NewFn->getIntrinsicID()) {
   default: {
     DefaultCase();
     return;
   }
   case Intrinsic::arm_neon_vst1:

llvm/test/Assembler/autoupgrade-invalid-name-mangling.ll

+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; CHECK: Intrinsic called with incompatible signature
+; CHECK-NEXT: %reg = call i32 @llvm.read_register.i64(
+; CHECK: Invalid user of intrinsic instruction!
+; CHECK-NEXT: %reg = call i32 @llvm.read_register.i64(
+define i32 @read_register_missing_mangling() {
+  %reg = call i32 @llvm.read_register(metadata !0)
+  ret i32 %reg
+}
+
+declare i64 @llvm.read_register(metadata)
+
+!0 = !{!"foo"}