[llvm] [indvars] Missing variables at Og: (PR #69920)

[Jeremy Morse via llvm-commits]

jmorse wrote:

Hmmm -- this started out as a patch that was to be for loop-deletion, it looks like I missed that this was firing at all times during indvars, that's my bad. The original rationale for all of this was that when loops are completely erased, we lose everything to do with debug-info that happened in the loop, which has the potential to mislead the developer. With that in mind, I believe the patch is collecting the correct information, just it's installing it at the wrong time. We should thus (IMO) be storing that information until loop-deletion commits to deleting a loop, at which point we definitely need to add intrinsics to the exit blocks.

Just to zip through Nikita's comments,

> you will be adding these debug values even if the IV does not actually get dropped

Yup, that's definitely broken,

> It will only recognize some "canonical" induction variable, but a loop may have many IVs.

Alas yes; as debug-info is to a large extent "best effort", we don't guarantee that it's always 100% completely correct, and there are some scenarios where we just bail out and say "that's too hard to solve". Seeing how deleting intrinsics is always unsafe (and is what happens on loop deletion today), it's better to have one patch that covers 90% of scenarios now than one that covers 100% later.

> If I'm reading this correctly, for the same reason as the previous point, you will be adding these debug values even if the IV does not actually get dropped

Assuming this is the `RewritePhiSet.empty()` portion -- there might be no users of the induction variable outside the loop, but we still have a debug-info interest in describing the exit value after the loop exits. Although: only if the loop and it's intrinsics get deleted.

https://github.com/llvm/llvm-project/pull/69920