[PATCH] D100137: [JumpThreading] merge debug info when merging select+br
Nick Desaulniers via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 8 17:31:46 PDT 2021
nickdesaulniers added a comment.
> I guess it'd probably be worth fixing that code to do something to add a valid location to those function calls too
Yeah, that sounds like the best long term solution to me; otherwise we'll be playing whack-a-mole with destructive transforms losing debug and results in confusing-for-developers error messages. When splitting a critical edge, what location do we use if neither contain debug info? If only one is missing debug info, I'd guess that `Instruction::applyMergedLocation` will pick the one existing one?
> then the sanitizer shouldn't rely on this guarantee
It's not so much the sanitizer relying on this, but the inliner. See http://reviews.llvm.org/rG93035c8f47589590b65041603524f48a7c007e1f which added it. LTO just exposes it since normally we can't/don't inline callees from compiler-rt, IIUC.
> then we probably want to add that to the verifier
And then have to fix all of the bugs it finds...? A worthy cause, but perhaps a severe undertaking (as you alluded to with "calls must have locations if the function they're in have a location").
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100137/new/
https://reviews.llvm.org/D100137
More information about the llvm-commits
mailing list