[PATCH] D136335: [Assignment Tracking Analysis][5/*] Tests

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

I can't really work out what untagged-store-frag.ll is testing as I don't get the notation, alas.



================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/diamond-3.ll:33-36
+;; === TODO / WISHLIST ===
+; LEBAL-KCEHC: bb.2.if.end:
+; KCEHC:         CALL64pcrel32 target-flags(x86-plt) @es
+; KCEHC:         DBG_VALUE %stack.0.a.addr, $noreg, ![[A]], !DIExpression(DW_OP_deref)
----------------
Orlando wrote:
> jmorse wrote:
> > I endorse the syntax; if this isn't happening, isn't that a regression from LowerDbgDeclare? debug-info-on-edges is a serious problem that we don't have a solution for right now sadly.
> Yeah it is a regression. I think TODO / WISHLIST should probably be renamed FIXME to indicate this. This is on my mental TODO list but I should add this to the actual TODO list in the documentation - leaving this comment as "not done" as a reminder to do that before landing.
Cool; I think we're alright landing things + evaluating so long as we've got this limitation on the radar. It's not like `LowerDbgDeclare` is perfect anyway.


================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/loop-sink.ll:57
+while.end:                                        ; preds = %while.body, %entry
+  %storemerge.lcssa = phi i32 [ %a, %entry ], [ %b, %while.body ]
+  store i32 %storemerge.lcssa, ptr %a.addr, align 4, !DIAssignID !20
----------------
Orlando wrote:
> jmorse wrote:
> > I had the slight expectation that this was going to get a dbg.value attached to it, this is deliberately dropped then?
> As in by the analysis pass (as a DBG_VALUE/INSTR_REF), or in this IR? Wasn't there a patch that stopped `insertDebugValuesForPHIs` being called by LCSSA maybe a year or so ago?
Yeah, but IIRC that was only for PHIs with single operands -- the loop here doesn't dominate the exit block, thus the PHI is relevant. Prodding this (full marks for including original source) with clang-15 shows it's not a regression. I'll write this down as something to examine later I guess. I think in the general case LiveDebugValues will recover this anyway.


================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/order-of-defs.ll:11
+
+; CHECK:      DBG_VALUE $esi, $noreg, ![[B]], !DIExpression(), debug-location !20
+; CHECK-NEXT: DBG_VALUE $edx, $noreg, ![[C]], !DIExpression(), debug-location !20
----------------
`!20` needs to be a captured variable


================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/remove-redundant-defs-to-prevent-reordering.ll:20
+;; }
+;; An `dbg.assign(undef, ...)` has been added by hand in if.end.
+
----------------
Not sure if it's "A" or "An", depends how you pronounce it I guess


================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/remove-undef-fragment.ll:72
+  %1 = extractvalue { <2 x float>, <2 x float> } %call, 1
+  %add.i = fadd float undef, 0.000000e+00
+  call void @llvm.dbg.assign(metadata float %add.i, metadata !11, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32), metadata !14, metadata ptr undef, metadata !DIExpression()), !dbg !15
----------------
You'll make the undef-to-poison people happy if you don't add this in a test.


================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll:4-5
+
+;; Hand-written to test untagged store handling on a simple case. Here's what
+;; we're looking at in the IR:
+
----------------
>From a standing start, I don't know what mem(a) and dbg(a) represent -- presumably the in-memory-or-in-value result computed by assignment tracing? But then the comment on 2. conflicts with that? 


================
Comment at: llvm/test/DebugInfo/assignment-tracking/X86/use-known-value-at-early-mem-def.ll:5
+
+;; Check that sandwitching instructions between a linked store and dbg.assign
+;; results in a dbg.value(prev_value) being inserted at the store, and a
----------------



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

https://reviews.llvm.org/D136335