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

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

Orlando added inline comments.


================
Comment at: llvm/lib/CodeGen/RemoveRedundantDebugValues.cpp:112
+    }
+
+    if (MI.isMetaInstruction())
----------------
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?


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

https://reviews.llvm.org/D105280