[PATCH] D91711: SCEV add function to see if SCEVUnknown is null
Markus Lavin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 26 07:37:51 PST 2020
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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the llvm-commits