[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