[PATCH] D140006: fix jump threading failing to update cloned dbg.values

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

Orlando added a comment.

This looks good but there's an edge case to consider:

`dbg.value` intrinsics can be associated with more than one value. I don't think this patch would update `%b` if it had an entry in `ValueMapping` for the intrinsic:

  call void @llvm.dbg.value(metadata !DIArgList(i32 %a, i32 %b), metadata !n, metadata !DIExpression(DW_OP_arg, 0, DW_OP_arg, 1, DW_OP_plus, DW_OP_stack_value)))

You can iterate over the values using `DbgVariableIntrinsic::location_ops()` and you can replace them using `DbgVariableIntrinsic::replaceVariableLocationOp`.

It may be acceptable to leave that as a "TODO" item - I'll leave that up to @jmorse. If it's decided to leave the patch as then please take a look at the inline comment I've added.

Thanks!



================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:2099
+    if (I != ValueMapping.end()) {
+      NewInst->setOperand(
+          0, MetadataAsValue::get(DbgInstruction->getContext(),
----------------
IMO we should use the higher-level  `DbgVariableIntrinsic::replaceVariableLocationOp(unsigned OpIdx, Value *NewValue)` here to replace the value rather than the more general/low-level method `setOperand`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140006