This is an alternative to D78398. In this diff we don't hide SalvageDebugInfoForDbgValues but we do simplify the salvaging interface and the algorithm in InstCombine.
I left some nits inline but otherwise I think this is good.
we -> We
Is this just printing the address of the clones? Shouldn't it be << *DIClones.back()?
While you're here you could updated salvaged -> Salvaged to match llvm style.
nit: I think this would be clearer, but not a strong opinion at all.
if (salvaged) return true; // <undef-code> return false;
Am I right in thinking that this move is just a result of no longer calling reverse(...) in the debuginfo sink update code in instcombine?
Sorry I didn't pick this up with my other comments. I think this should be auto *DIIClone (or DbgVariableIntrinsic *DIIClone) because DIIClones is SmallVector<DbgVariableIntrinsic *, 2>?
- Corrected comment
- Removed unnecessary reference in iteration
- Added missing pointer dereference
Are there any more suggestions please?
Yes I prefer this too.
Sorry for the delay, LGTM. Please wait 24h to see if anyone else has any further comments.