[llvm] [DebugInfo][InstCombine] When replacing bswap idiom, add DebugLoc to new insts (PR #114231)
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 4 05:22:22 PST 2024
SLTozer wrote:
> if we considered the case where multiple instructions were replaced with one instruction, I think our documented process is pretty clear that we should merge locations there, right?
Hm, you're right that this is the documented rule - I went into this thinking this case was more ambiguous, because in InstCombine that rule isn't always strictly followed: we perform peephole optimizations that may replace multiple instructions with a single instruction that uses the location of the final replaced instruction.
For this patch I'm happy to switch back to use merged locations to keep it consistent with the existing rules, but I think the rule should be different: my short proposal would be that when replacing a sequence of instructions, where there are no side-effects and only one instruction's output is being replaced by the new instruction(s), we should use the output instruction's location for the instruction(s) inserted to replace it. That's a discussion that should happen elsewhere (either an RFC or another review), but I think it's probably worth having. In the meanwhile, I'll update this patch!
https://github.com/llvm/llvm-project/pull/114231
More information about the llvm-commits
mailing list