[PATCH] D148020: [Assignment Tracking][SROA] Don't un-poison dbg.assigns using multiple loc ops

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

Orlando created this revision.
Orlando added a reviewer: jmorse.
Orlando added a project: debug-info.
Herald added a subscriber: hiraditya.
Herald added a project: All.
Orlando requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Some dbg.assigns using poison become un-poisoned in SROA. The reason this happens at all is because dbg.assigns linked to memory intrinsics use poison to indicate they can't describe the stored value, but the value becomes available after some optimisations. This needs reworking eventually, but for now we need to ensure that when it does occur we don't create invalid expressions.

D147312 <https://reviews.llvm.org/D147312> prevented this occuring when the dbg.assign uses DIArgLists, but that wasn't a complete fix. We also need to ensure we avoid un-poisoning when the existing expression uses more than one location operand (DW_OP_arg, n).


https://reviews.llvm.org/D148020

Files:
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/test/DebugInfo/Generic/assignment-tracking/sroa/fail-fragment.ll


Index: llvm/test/DebugInfo/Generic/assignment-tracking/sroa/fail-fragment.ll
===================================================================
--- llvm/test/DebugInfo/Generic/assignment-tracking/sroa/fail-fragment.ll
+++ llvm/test/DebugInfo/Generic/assignment-tracking/sroa/fail-fragment.ll
@@ -5,6 +5,10 @@
 ;; it used a fragment that can't be split (the first check directive below).
 ;; NOTE: If createFragmentExpression gets smarter it may be necessary to create
 ;; a new test case.
+;; In some cases a dbg.assign with a poison value is replaced with a non-poison
+;; value. This needs reworking, but as a stop-gap we need to ensure this
+;; doesn't cause invalid expressions to be created. Check we don't do it when
+;; the expression uses more than one location operand (DW_OP_arg n).
 
 ; CHECK: if.then:
 ; CHECK: dbg.value(metadata i32 poison, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
@@ -15,6 +19,9 @@
 ; CHECK: dbg.value(metadata i32 2, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
 ; CHECK: dbg.value(metadata i32 0, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32))
 
+; CHECK: if.inner:
+; CHECK: call void @llvm.dbg.value(metadata i32 poison, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value))
+
 ; CHECK: end:
 ; CHECK: dbg.value(metadata i32 %{{.*}}, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
 
@@ -37,6 +44,11 @@
 if.else:
   store i64 2, ptr %codepoint, align 4, !DIAssignID !28
   call void @llvm.dbg.assign(metadata i32 2, metadata !15, metadata !DIExpression(), metadata !28, metadata ptr %codepoint, metadata !DIExpression()), !dbg !26
+  br i1 %cmp, label %end, label %if.inner
+
+if.inner:
+  store i32 3, ptr %codepoint, align 4, !DIAssignID !29
+  call void @llvm.dbg.assign(metadata i32 poison, metadata !15, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value), metadata !29, metadata ptr %codepoint, metadata !DIExpression()), !dbg !26
   br label %end
 
 end:
@@ -63,3 +75,4 @@
 !26 = !DILocation(line: 0, scope: !18)
 !27 = distinct !DIAssignID()
 !28 = distinct !DIAssignID()
+!29 = distinct !DIAssignID()
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -302,7 +302,9 @@
     // This should be a very rare situation as it requires the value being
     // stored to differ from the dbg.assign (i.e., the value has been
     // represented differently in the debug intrinsic for some reason).
-    SetKillLocation |= DbgAssign->hasArgList() && Value;
+    SetKillLocation |=
+        Value && (DbgAssign->hasArgList() ||
+                  !DbgAssign->getExpression()->isSingleLocationExpression());
     if (SetKillLocation)
       NewAssign->setKillLocation();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148020.512462.patch
Type: text/x-patch
Size: 2942 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230411/677c73c6/attachment.bin>