[PATCH] D91711: SCEV add function to see if SCEVUnknown is null

[Markus Lavin via Phabricator via llvm-commits]

markus added a comment.

Right, there is nothing special with llvm.dbg.value besides that it is a debug intrinsic so it is not allowed to affect optimizations and the fact that such intrinsic uses another Value may not inhibit transformation on that Value. I just wanted to point it out since the situation that predecessors of an instruction get removed without the instruction itself is a bit unusual (but otherwise unimportant for this discussion).

Ok, so whenever an instruction gets deleted we should call `forgetValue` on it to make sure that ScalarEvolution clears any SCEV based on it from its cache. That also suggest that I am not allowed to store any SCEV but must always call `ScalarEvolution::getSCEV` (at least if I start making modifications to the IR)?

So in order to do https://reviews.llvm.org/D87494 we would need to introduce a new representation that is a more lightweight version of the SCEV (lets call it MySCEV). Before LSR starts its transformation we build a MySCEV based on the SCEV for each intrinsic and store away the former.

Later after transformation we again build MySCEVs based on the SCEV of phi-nodes and then do the comparison in the MySCEV domain.

Sure that would work but it seems a bit cumbersome to basically replicate what we already have (although it would be much more lightweight than a full SCEV). At least with a new representation there would be less restrictions on how it could be used but I am not sure reviewers would be too happy with the duplication.

But I don’t know. I am fine with either way (or simply just reverting https://reviews.llvm.org/D87494) as long as a sufficient number of people think it is a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91711