[PATCH] D105280: [2/2][RemoveRedundantDebugValues] Introduce a MIR pass that removes redundant DBG_VALUEs

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/RemoveRedundantDebugValues.cpp:112
+    }
+
+    if (MI.isMetaInstruction())
----------------
Orlando wrote:
> StephenTozer wrote:
> > This LGTM with this edit, unless I've made a mistake in believing it necessary. I believe this will be necessary to prevent incorrect debug info from being caused in the following case:
> > 
> > ```
> > 1  DBG_VALUE $rax, "x", DIExpression()
> >    ...
> > 2  DBG_VALUE_LIST "x", DIExpression(...), $rax, $rbx
> >    ...
> > 3  DBG_VALUE $rax, "x", DIExpression()
> > ```
> > 
> > Currently we would ignore the `DBG_VALUE_LIST` entirely, and so 3 would falsely appear to be redundant due to being a repeat of 1. Killing any existing mapping for the variable of a `DBG_VALUE_LIST` is the simplest way to ensure this doesn't happen, without needing a full implementation for list debug values.
> FWIW I agree with @StephenTozer and just wanted to add: please can you add a test for this too when you update the code?
Sure! Thanks! Good catch!

In addition, we should stop tracking a variable in this case as well (if `VMI != VariableMap.end()`):
   if (!Loc.isReg())
     continue

I'll make tests for this before updating the patch.


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

https://reviews.llvm.org/D105280