[PATCH] D106659: [DebugInfo][InstrRef] Handle llvm.frameaddress intrinsics gracefully

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: Orlando, TWeaver, StephenTozer.
Herald added a subscriber: hiraditya.
jmorse requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

When working out which instruction defines a value, the instruction-referencing variable location code has a few special cases for physical registers:

- Arguments are never defined by instructions,
- Constant physical registers always read the same value and are never def'd.

This patch adds a third case for the llvm.frameaddress intrinsics: you can read the framepointer in any block if you so choose, and use it as a variable location, as shown in the attached patch.

This rather violates one of the assumptions behind instruction referencing, that llvm-ir shouldn't be able to read from an arbitrary register at some arbitrary point in the program. The solution for now is to just emit a DBG_PHI that reads the register value: this works, but if we wanted to do something clever with DBG_PHIs in the future then this would probably get in the way. As it stands, this patch avoids a crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106659

Files:
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll


Index: llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll
===================================================================
--- /dev/null
+++ llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll
@@ -0,0 +1,41 @@
+; RUN: llc %s -o - -start-after=codegenprepare -stop-before=finalize-isel \
+; RUN:     -mtriple=x86_64-unknown-unknown -experimental-debug-variable-locations \
+; RUN: | FileCheck %s
+
+; Test for a crash / weird behaviour when llvm.frameaddress.* is called.
+; Today, as a concession, we emit a DBG_PHI allowing the frame register value
+; to be read, but it could be expressed in other ways. Check that this works
+; outside of the entry block.
+
+; CHECK-LABEL: bb.1.notentry:
+; CHECK: DBG_PHI $rbp
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare i8 *@llvm.frameaddress.p0i8(i32)
+
+ ; Function Attrs: mustprogress nofree nosync nounwind sspstrong uwtable
+define hidden i8 * @foo() !dbg !7 {
+ entry:
+  br label  %notentry
+
+notentry:
+   %0 = tail call i8* @llvm.frameaddress.p0i8(i32 0), !dbg !12
+   call void @llvm.dbg.value(metadata i8* %0, metadata !11, metadata !DIExpression()), !dbg !12
+  ret i8 *%0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "foo.cpp", directory: ".")
+!2 = !DIBasicType(name: "int", size: 8, encoding: DW_ATE_signed)
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!2, !2}
+!10 = !{!11}
+!11 = !DILocalVariable(name: "baz", scope: !7, file: !1, line: 7, type: !2)
+!12 = !DILocation(line: 10, scope: !7)
Index: llvm/lib/CodeGen/MachineFunction.cpp
===================================================================
--- llvm/lib/CodeGen/MachineFunction.cpp
+++ llvm/lib/CodeGen/MachineFunction.cpp
@@ -1196,6 +1196,14 @@
     // We can produce a DBG_PHI that identifies the constant physreg. Doesn't
     // matter where we put it, as it's constant valued.
     assert(CurInst->isCopy());
+  } else if (State.first == TRI.getFrameRegister(*this)) {
+    // LLVM IR is allowed to read the framepointer by calling a
+    // llvm.frameaddress.* intrinsic. We can support this by emitting a
+    // DBG_PHI $fp. This isn't ideal, because it extends the behaviours /
+    // position that DBG_PHIs appear at, limiting what can be done later.
+    // TODO: see if there's a better way of expressing these variable
+    // locations.
+    ;
   } else {
     // Assert that this is the entry block. If it isn't, then there is some
     // code construct we don't recognise that deals with physregs across


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D106659.361174.patch
Type: text/x-patch
Size: 3039 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210723/f303adf8/attachment.bin>