[PATCH] D144793: [DebugInfo] Upgrade `dbg.addr` to `dbg.value`

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

I just have a couple of nits/questions (most inline), otherwise SGTM.

Was `llvm/test/Bitcode/upgrade-dbg-addr.ll.bc` added by accident?



================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:4140-4141
+    StringRef Name = F->getName();
+    Name = Name.substr(5); // Strip llvm
+    // Upgrade `dbg.addr` to `dbg.value` with `DW_OP_deref`
+    if (Name.startswith("dbg.addr")) {
----------------
nit: missing full stops for the comments.


================
Comment at: llvm/test/Bitcode/upgrade-dbg-addr.ll:11
+  ; CHECK-NOT: call void @llvm.dbg.addr
+  ; CHECK: call void @llvm.dbg.value(metadata ptr %num.addr, metadata !16, metadata !DIExpression(DW_OP_deref))
+  call void @llvm.dbg.addr(metadata ptr %num.addr, metadata !16, metadata !DIExpression())
----------------
Better to avoid hard-coding metadata in the CHECK directives (I've not used the `[[#]]` syntax before to match numbers but iiuc it should work here).



================
Comment at: llvm/test/Bitcode/upgrade-dbg-addr.ll:18
+; CHECK: declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare void @llvm.dbg.addr(metadata, metadata, metadata)
----------------
Did you exclude the metadata definitions on purpose and does the test work like this?

It seems odd to me, but I've not looked at the upgrade tests before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144793