[PATCH] D129636: Fix a LSR debug invariance issue

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

> Did you consider that forgetValue in fact does not only forget the value you are aiming, but also all its users recursively? Not SCEV users (they likely don't exist), but instruction users.

Ah, that rather sounds like a blocker -- if there's no way to limit what's forgotten to "just the things unique to this Value", it's going to be impossible to peel apart "what was important to debug-info" and "what was important to codegen". A quick look at the SCEV methods suggests that plumbing in an argument flag is going to be pretty invasive, `ScalarEvolution::getSCEV` fans out to a lot of functions with a lot of logic.

Stephen wrote:

> Though I'm also surprised that just adding something to the cache is modifying the output

I imagine you might find a different way expanding expressions for Values, depending on which PHIs you look at first, which changes if you look at the dbg.values first.

> The best alternative I can suggest is to create a new ScalarEvolution instance and use it for dbg value queries. It should be good enough if the only purpose of that is to find undefs.

I'm in two minds about this -- it solves all the problems raised (obviously), on the other hand I imagine it doubles the amount of work done, which isn't ideal. However, I think the reason why we're using ScalarEvolution in this way is because all the PHIs might get replaced during loop-strength-reduction, and we need to use ScalarEvolution's map of old-to-new PHIs to reconstruct the dbg.values operands -- is that right @chrisjackson ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129636/new/

https://reviews.llvm.org/D129636