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

7

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

7

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

             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;
 

     }
     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;

     CI->eraseFromParent();
     return;
 
+  case Intrinsic::ptr_annotation:
+    // Upgrade from versions that lacked the annotation attribute argument.
+    assert(CI->getNumArgOperands() == 4);
+    // 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);
+    // 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)});

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

+; Test upgrade of ptr.annotation intrinsics.
+; 
+; RUN: llvm-dis < %s.bc | FileCheck %s
+
+; Unused return values
+define void @f1() {
+  %t0 = call i8* @llvm.ptr.annotation.p0i8(i8* undef, i8* undef, i8* undef, i32 undef)
+;CHECK:  call i8* @llvm.ptr.annotation.p0i8(i8* undef, i8* undef, i8* undef, i32 undef, i8* null)
+
+  %t1 = call i16* @llvm.ptr.annotation.p0i16(i16* undef, i8* undef, i8* undef, i32 undef)
+;CHECK:  call i16* @llvm.ptr.annotation.p0i16(i16* undef, i8* undef, i8* undef, i32 undef, i8* null)
+
+  %t2 = call i256* @llvm.ptr.annotation.p0i256(i256* undef, i8* undef, i8* undef, i32 undef)
+;CHECK:  call i256* @llvm.ptr.annotation.p0i256(i256* undef, i8* undef, i8* undef, i32 undef, 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() {
+  call void @llvm.var.annotation(i8* undef, i8* undef, i8* undef, i32 undef)
+;CHECK:  call void @llvm.var.annotation(i8* undef, i8* undef, i8* undef, i32 undef, 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*)