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

[J. Ryan Stinnett via Phabricator via llvm-commits]

jryans marked 3 inline comments as done.
jryans added a comment.

In D144793#4154732 <https://reviews.llvm.org/D144793#4154732>, @Orlando wrote:

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

No, bitcode tests seem to use a pattern of calling `llvm-dis` on a separate `.bc` file, so that's what I did here. The `.ll` file is not the test input like it usually is in most other tests, so that's why e.g. the metadata is not needed. I suppose only the `CHECK` lines are actually required, but other files seemed to include some amount IR in the `.ll` part for explanatory reasons I guess.



================
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())
----------------
Orlando wrote:
> 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).
> 
Ah, good tip, done!


================
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)
----------------
Orlando wrote:
> 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.
The `.ll` content is not that meaningful here because only the `.bc` is actually used as an input (unlike most other tests). Bitcode tests seem to use this pattern regularly from what I could see.

In any case, yes, the test does pass like this. 🙂 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144793