[DebugInfo] Upgrade `dbg.addr` to `dbg.value`

[jryans]

Summary

As part of removing dbg.addr, this upgrades any calls to dbg.value with
DW_OP_deref prepended onto the value expression.

Part of dbg.addr removal
Discussed in https://discourse.llvm.org/t/what-is-the-status-of-dbg-addr/62898

Depends on D144792

[Orlando]

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

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

llvm/lib/IR/AutoUpgrade.cpp

4141–4142

nit: missing full stops for the comments.

llvm/test/Bitcode/upgrade-dbg-addr.ll

12

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).

19

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.

[jryans]

In 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.

llvm/test/Bitcode/upgrade-dbg-addr.ll

12

Ah, good tip, done!

19

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. 🙂

[jryans]

Fixed comments and test syntax.

[Orlando]

Apologies for adding an additional round of comments - these should / could have come in the first review.

llvm/lib/IR/AutoUpgrade.cpp

4146

Actually, thinking about it, shouldn't the DW_OP_deref be appended rather than prepended? Before converting the dbg.addr to a dbg.value, the expression modified the address of the variable. In order for that to remain true the deref needs to be added to the end of the expression.

llvm/test/Bitcode/upgrade-dbg-addr.ll

12

It just occurred to me that it might be beneficial to include an existing expression (e.g.DW_OP_plus, DW_OP_constu, 0 or something) to test the pre/appended-ness of it.

19

Oh I see. Ok cool, that makes sense to me.

[jryans]

llvm/lib/IR/AutoUpgrade.cpp

4146

Ah yes, good catch! Not sure why I convinced myself "prepend" was the thing to do... We want the fully transformed address from the dbg.addr expression, so the deref should go at the end.

I'll update this and improve the test coverage to match.

llvm/test/Bitcode/upgrade-dbg-addr.ll

12

Right, good idea, I will add that.

[jryans]

Switched to appending and extended test to ensure that's indeed what happens.

[jryans]

llvm/test/Bitcode/upgrade-dbg-addr.ll

19

Now that I'm having to modify the test, I think it's better to have the full .ll file in tree for easier tweaking when needed. I'll keep the .bc as the test input though, since that seems to be the pattern here.

[Orlando]

Thanks - LGTM!

llvm/lib/IR/AutoUpgrade.cpp

 //===-- AutoUpgrade.cpp - Implement auto-upgrade helper functions ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the auto-upgrade helper functions.
 // This is where deprecated IR intrinsics and other IR features are updated to
 // current specifications.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsAArch64.h"
 #include "llvm/IR/IntrinsicsARM.h"
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/TargetParser/Triple.h"
 #include <cstring>
 using namespace llvm;
 
if (Name.startswith("cttz.") && F->arg_size() == 1) {
       rename(F);
       NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::cttz,
                                         F->arg_begin()->getType());
       return true;
     }
     break;
   }
   case 'd': {
+    if (Name == "dbg.addr") {
+      rename(F);
+      NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::dbg_value);
+      return true;
+    }
     if (Name == "dbg.value" && F->arg_size() == 4) {
       rename(F);
       NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::dbg_value);
       return true;
     }
     break;
   }
   case 'e': {

   case Intrinsic::ctpop:
     NewCall = Builder.CreateCall(NewFn, {CI->getArgOperand(0)});
     break;
 
   case Intrinsic::convert_from_fp16:
     NewCall = Builder.CreateCall(NewFn, {CI->getArgOperand(0)});
     break;
 
-  case Intrinsic::dbg_value:
+  case Intrinsic::dbg_value: {
+    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")) {
+      DIExpression *Expr = cast<DIExpression>(
+          cast<MetadataAsValue>(CI->getArgOperand(2))->getMetadata());
+      Expr = DIExpression::append(Expr, dwarf::DW_OP_deref);
+      NewCall =
+          Builder.CreateCall(NewFn, {CI->getArgOperand(0), CI->getArgOperand(1),
+                                     MetadataAsValue::get(C, Expr)});
+      break;
+    }
+
     // Upgrade from the old version that had an extra offset argument.
     assert(CI->arg_size() == 4);
     // Drop nonzero offsets instead of attempting to upgrade them.
     if (auto *Offset = dyn_cast_or_null<Constant>(CI->getArgOperand(1)))
       if (Offset->isZeroValue()) {
         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.
     if (CI->arg_size() != 4) {
       DefaultCase();
       return;
     }
 

llvm/test/Bitcode/upgrade-dbg-addr.ll

+; Test upgrade of dbg.addr intrinsics into dbg.value with DW_OP_deref appended
+;
+; RUN: llvm-dis < %s.bc | FileCheck %s
+; RUN: verify-uselistorder < %s.bc
+
+define i32 @example(i32 %num) {
+entry:
+  %num.addr = alloca i32, align 4
+  store i32 %num, ptr %num.addr, align 4
+  ; CHECK-NOT: call void @llvm.dbg.addr
+  ; CHECK: call void @llvm.dbg.value(metadata ptr %num.addr, metadata ![[#]], metadata !DIExpression(DW_OP_plus_uconst, 0, DW_OP_deref))
+  call void @llvm.dbg.addr(metadata ptr %num.addr, metadata !16, metadata !DIExpression(DW_OP_plus_uconst, 0)), !dbg !17
+  %0 = load i32, ptr %num.addr, align 4
+  ret i32 %0
+}
+
+; CHECK: declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare void @llvm.dbg.addr(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
+!1 = !DIFile(filename: "/app/example.c", directory: "/app")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = distinct !DISubprogram(name: "example", scope: !11, file: !11, line: 1, type: !12, scopeLine: 1, unit: !0, retainedNodes: !15)
+!11 = !DIFile(filename: "example.c", directory: "/app")
+!12 = !DISubroutineType(types: !13)
+!13 = !{!14}
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!15 = !{}
+!16 = !DILocalVariable(name: "num", arg: 1, scope: !10, file: !11, line: 1, type: !14)
+!17 = !DILocation(line: 1, column: 17, scope: !10)

llvm/test/Bitcode/upgrade-dbg-value.ll

-; Test upgrade of dbg.dvalue intrinsics with offsets.
+; Test upgrade of dbg.value intrinsics with offsets.
 ;
 ; RUN: llvm-dis < %s.bc | FileCheck %s
 ; RUN: verify-uselistorder < %s.bc
 
 define void @f() !dbg !3 {
 entry:
   ; CHECK-NOT: call void @llvm.dbg.value
   ; CHECK: call void @llvm.dbg.value(metadata i32 42, metadata !8, metadata !DIExpression())