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

Thu Jan 23 10:19:01 PST 2020

TWeaver 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
----------------
djtodoro wrote:
> 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)
great, that works for me.

It was always a case of remove the check or change the instructions.

happy to have gotten a more definitive decision on which one was the best approach.

I'll get this changed out re-uploaded. an official LGTM would be great and I'll get his pushed today.

thanks again!
Tom W

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

https://reviews.llvm.org/D73210