Add auto-upgrade support for annotation intrinsics

[andrew.w.kaylor]

Summary

The llvm.ptr.annotation and llvm.var.annotation intrinsics were changedsince the 11.0 release to add an additional parameter. This patch auto-upgrades IR containing the four-parameter versions of these intrinsics, adding a null pointer as the fifth argument.

[aaron.ballman]

Adding another potential reviewer.

[andrew.w.kaylor]

@pcc CODE_OWNERS.txt says that you own the bitcode. Can you help with this review?

[Tyker]

This seems good to me but i am not familiar the the auto-upgrader.

[Tyker]

llvm/lib/IR/AutoUpgrade.cpp

3759

since the return type of annotation didn't change i am not sure this is necessary.

[Tyker]

llvm/lib/IR/AutoUpgrade.cpp

3759

i am not sure the bitcast is necessary.

[fhahn]

This looks reasonable, but the bit cast creation should be dropped, if it is not necessary.

[andrew.w.kaylor]

llvm/lib/IR/AutoUpgrade.cpp

3759

Yes, you're right. I was thinking about the fact that the function type changed, but that doesn't affect the return value type, so the bitcast is not needed.

[andrew.w.kaylor]

Removed unnecessary bitcast

[andrew.w.kaylor]

Ping

[fhahn]

I just checked, and it appears the langref still only documents the 4 argument variants (https://llvm.org/docs/LangRef.html#llvm-ptr-annotation-intrinsic). I think those need to be updated as well?

[andrew.w.kaylor]

In D95993#2579624, @fhahn wrote:

I just checked, and it appears the langref still only documents the 4 argument variants (https://llvm.org/docs/LangRef.html#llvm-ptr-annotation-intrinsic). I think those need to be updated as well?

There's a separate patch for the LangRef here: https://reviews.llvm.org/D96646

[fhahn]

LGTM with the additional comments & assuming passing null is safe.

llvm/lib/IR/AutoUpgrade.cpp

3751

nit: might be good to add a message to the assert, like && "old version of the intrinsic took 4 arguments"

llvm/test/Bitcode/upgrade-ptr-annotation.ll

8

can you pass distinct arguments to the call? Otherwise we are not properly checking if we forward the correct arguments.

llvm/test/Bitcode/upgrade-var-annotation.ll

8

can you pass distinct arguments to the call? Otherwise we are not properly checking if we forward the correct arguments.

[andrew.w.kaylor]

Addressed review feedback

llvm/lib/IR/AutoUpgrade.cpp

if (Name == "prefetch") {
       // Handle address space overloading.
       Type *Tys[] = {F->arg_begin()->getType()};
       if (F->getName() != Intrinsic::getName(Intrinsic::prefetch, Tys)) {
         rename(F);
         NewFn =
             Intrinsic::getDeclaration(F->getParent(), Intrinsic::prefetch, Tys);
         return true;
       }
+    } else if (Name.startswith("ptr.annotation.") && F->arg_size() == 4) {
+      rename(F);
+      NewFn = Intrinsic::getDeclaration(F->getParent(),
+                                        Intrinsic::ptr_annotation,
+                                        F->arg_begin()->getType());
+      return true;
     }
     break;
 
   case 's':
     if (Name == "stackprotectorcheck") {
       NewFn = nullptr;
       return true;
     }
     break;
 
+  case 'v': {
+    if (Name == "var.annotation" && F->arg_size() == 4) {
+      rename(F);
+      NewFn = Intrinsic::getDeclaration(F->getParent(),
+                                        Intrinsic::var_annotation);
+      return true;
+    }
+    break;
+  }
+
   case 'x':
     if (UpgradeX86IntrinsicFunction(F, Name, NewFn))
       return true;
   }
   // Remangle our intrinsic since we upgrade the mangling
   auto Result = llvm::Intrinsic::remangleIntrinsicFunction(F);
   if (Result != None) {
     NewFn = Result.getValue();
if (auto *Offset = dyn_cast_or_null<Constant>(CI->getArgOperand(1)))
         NewCall = Builder.CreateCall(
             NewFn,
             {CI->getArgOperand(0), CI->getArgOperand(2), CI->getArgOperand(3)});
         break;
       }
     CI->eraseFromParent();
     return;
 
+  case Intrinsic::ptr_annotation:
+    // Upgrade from versions that lacked the annotation attribute argument.
+    assert(CI->getNumArgOperands() == 4 &&
+           "Before LLVM 12.0 this intrinsic took four arguments");
+    // Create a new call with an added null annotation attribute argument.
+    NewCall = Builder.CreateCall(
+        NewFn,
+        {CI->getArgOperand(0), CI->getArgOperand(1), CI->getArgOperand(2),
+         CI->getArgOperand(3), Constant::getNullValue(Builder.getInt8PtrTy())});
+    NewCall->takeName(CI);
+    CI->replaceAllUsesWith(NewCall);
+    CI->eraseFromParent();
+    return;
+
+  case Intrinsic::var_annotation:
+    // Upgrade from versions that lacked the annotation attribute argument.
+    assert(CI->getNumArgOperands() == 4 &&
+           "Before LLVM 12.0 this intrinsic took four arguments");
+    // Create a new call with an added null annotation attribute argument.
+    NewCall = Builder.CreateCall(
+        NewFn,
+        {CI->getArgOperand(0), CI->getArgOperand(1), CI->getArgOperand(2),
+         CI->getArgOperand(3), Constant::getNullValue(Builder.getInt8PtrTy())});
+    CI->eraseFromParent();
+    return;
+
   case Intrinsic::x86_xop_vfrcz_ss:
   case Intrinsic::x86_xop_vfrcz_sd:
     NewCall = Builder.CreateCall(NewFn, {CI->getArgOperand(1)});
     break;
 
   case Intrinsic::x86_xop_vpermil2pd:
   case Intrinsic::x86_xop_vpermil2ps:
   case Intrinsic::x86_xop_vpermil2pd_256:

llvm/test/Bitcode/upgrade-ptr-annotation.ll

+; Test upgrade of ptr.annotation intrinsics.
+;
+; RUN: llvm-dis < %s.bc | FileCheck %s
+
+; Unused return values
+; The arguments passed to the intrinisic wouldn't normally be arguments to
+; the function, but that makes it easier to test that they are handled
+; correctly.
+define void @f1(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3) {
+;CHECK: @f1(i8* [[ARG0:%.*]], i8* [[ARG1:%.*]], i8* [[ARG2:%.*]], i32 [[ARG3:%.*]])
+  %t0 = call i8* @llvm.ptr.annotation.p0i8(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3)
+;CHECK:  call i8* @llvm.ptr.annotation.p0i8(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+
+  %arg0_p16 = bitcast i8* %arg0 to i16*
+  %t1 = call i16* @llvm.ptr.annotation.p0i16(i16* %arg0_p16, i8* %arg1, i8* %arg2, i32 %arg3)
+;CHECK:  [[ARG0_P16:%.*]] = bitcast
+;CHECK:  call i16* @llvm.ptr.annotation.p0i16(i16* [[ARG0_P16]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+
+  %arg0_p256 = bitcast i8* %arg0 to i256*
+  %t2 = call i256* @llvm.ptr.annotation.p0i256(i256* %arg0_p256, i8* %arg1, i8* %arg2, i32 %arg3)
+;CHECK:  [[ARG0_P256:%.*]] = bitcast
+;CHECK:  call i256* @llvm.ptr.annotation.p0i256(i256* [[ARG0_P256]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+  ret void
+}
+
+; Used return values
+define i16* @f2(i16* %x, i16* %y) {
+  %t0 = call i16* @llvm.ptr.annotation.p0i16(i16* %x, i8* undef, i8* undef, i32 undef)
+  %t1 = call i16* @llvm.ptr.annotation.p0i16(i16* %y, i8* undef, i8* undef, i32 undef)
+  %cmp = icmp ugt i16* %t0, %t1
+  %sel = select i1 %cmp, i16* %t0, i16* %t1
+  ret i16* %sel
+; CHECK:  [[T0:%.*]] = call i16* @llvm.ptr.annotation.p0i16(i16* %x, i8* undef, i8* undef, i32 undef, i8* null)
+; CHECK:  [[T1:%.*]] = call i16* @llvm.ptr.annotation.p0i16(i16* %y, i8* undef, i8* undef, i32 undef, i8* null)
+; CHECK:  %cmp = icmp ugt i16* [[T0]], [[T1]]
+; CHECK:  %sel = select i1 %cmp, i16* [[T0]], i16* [[T1]]
+; CHECK:  ret i16* %sel
+}
+
+declare i8*   @llvm.ptr.annotation.p0i8(i8*, i8*, i8*, i32)
+; CHECK: declare i8*   @llvm.ptr.annotation.p0i8(i8*, i8*, i8*, i32, i8*)
+declare i16*  @llvm.ptr.annotation.p0i16(i16*, i8*, i8*, i32)
+; CHECK: declare i16*   @llvm.ptr.annotation.p0i16(i16*, i8*, i8*, i32, i8*)
+declare i256* @llvm.ptr.annotation.p0i256(i256*, i8*, i8*, i32)
+; CHECK: declare i256*   @llvm.ptr.annotation.p0i256(i256*, i8*, i8*, i32, i8*)

llvm/test/Bitcode/upgrade-var-annotation.ll

+; Test upgrade of var.annotation intrinsics.
+;
+; RUN: llvm-dis < %s.bc | FileCheck %s
+
+
+define void @f(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3) {
+;CHECK: @f(i8* [[ARG0:%.*]], i8* [[ARG1:%.*]], i8* [[ARG2:%.*]], i32 [[ARG3:%.*]])
+  call void @llvm.var.annotation(i8* %arg0, i8* %arg1, i8* %arg2, i32 %arg3)
+;CHECK:  call void @llvm.var.annotation(i8* [[ARG0]], i8* [[ARG1]], i8* [[ARG2]], i32 [[ARG3]], i8* null)
+  ret void
+}
+
+; Function Attrs: nofree nosync nounwind willreturn
+declare void @llvm.var.annotation(i8*, i8*, i8*, i32)
+; CHECK: declare void @llvm.var.annotation(i8*, i8*, i8*, i32, i8*)