[PATCH] D73210: [OPT_DPG][LIVEDEBUGVALUES] Teach Live Debug Values About Meta Instructions

Djordje Todorovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 23 01:16:06 PST 2020


djtodoro added inline comments.


================
Comment at: llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir:106
     DBG_VALUE $ebx, $noreg, !17, !DIExpression(), debug-location !20
-    renamable $edi = KILL $edi, implicit-def $rdi
+    $edi = MOV32ri 5, debug-location !22
     DBG_VALUE $edi, $noreg, !16, !DIExpression(), debug-location !20
----------------
vsk wrote:
> TWeaver wrote:
> > djtodoro wrote:
> > > What is the intention here?
> > The KILL was being picked up as a def in the test and was causing a DBG_VALUE instruction to be created. This was tested for in the check list. Ignoring KILL instructions means that livedebugvalues wasn't creating the second entry DBG_VALUE for ARG_A anymore. 
> > 
> > In order to further preserve what I figured was intended behaviour in the test I changed it to a concrete register def.
> > 
> > The original entry value patch, SFAIK, had nothing to do with KILL/meta instructions. I figured the KILL instruction was there by coincidence rather than intention.
> > 
> > happy to reconsider this if that's not the case.
> > 
> > Cheers
> > Tom W
> This seems like a reasonable change, we just want a real def here. We could just remove the second CHECK for ARG_A, but that arguably drops code coverage.
I see the result, but I don't think we should change the code generated here, since it was produced from the C code provided in the source code of the test. Now we have the two real identical defs for the `$edi`..
Anyhow, the main intention in the test was to test the `ARG_B` doesn't have the `entry_val` generated, since its entry value gets modified, and it seems reasonable to me to just remove the first `entry_val` check for the `ARG_A`.
@vsk Sounds reasonable?

@TWeaver Thanks for this! Besides this, lgtm! (but I am not listed as a reviewer)


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

https://reviews.llvm.org/D73210





More information about the llvm-commits mailing list