[PATCH] D95993: Add auto-upgrade support for annotation intrinsics

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:3751
+    // Upgrade from versions that lacked the annotation attribute argument.
+    assert(CI->getNumArgOperands() == 4);
+    // Create a new call with an added null annotation attribute argument.
----------------
nit: might be good to add a message to the assert, like `&& "old version of the intrinsic took 4 arguments"`


================
Comment at: llvm/test/Bitcode/upgrade-ptr-annotation.ll:7
+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)
----------------
can you pass distinct arguments to the call? Otherwise we are not properly checking if we forward the correct arguments.


================
Comment at: llvm/test/Bitcode/upgrade-var-annotation.ll:7
+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)
----------------
can you pass distinct arguments to the call? Otherwise we are not properly checking if we forward the correct arguments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95993/new/

https://reviews.llvm.org/D95993