[PATCH] D134739: [SCEVExpander] Check expr is available for traversed InsterPts.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 463981.
fhahn added a comment.
Herald added a subscriber: pcwang-thead.



In D134739#3819071 <https://reviews.llvm.org/D134739#3819071>, @efriedma wrote:

> Maybe we should restructure the InsertedExpressions cache so it's more likely to consistently hit?
>
> Insertion points fall into the following categories:
>
> - A passed-in arbitrary point.
> - The beginning of a basic block.
> - The end of a basic block.
>
> The values are then adjusted in some cases: if the insertion point points at an inserted instruction, we adjust forward to try to reuse those instructions.
>
> For the "beginning of a basic block" case, it doesn't really make sense to track the exact instruction where we can actually insert instructions; all that matters is that it's the "beginning".  If we fix that, it should ensure we hit the cache reliably for those cases.
>
> Doing multiple cache lookups like the current patch may sort of work in this case, but it's not clear in general why we would expect the unadjusted insertion point to be in the cache.

I took another closer look at where the LCSSA phi nodes should be cached. I restructured the code to move LCSSA fixup to ::expand() from ::expandCodeForImpl. This has the advantage that we directly preserve LCSSA nodes here instead of relying on doing so in rememberInstruction. It also ensures that we don't add the non-LCSSA-safe value to InsertedExpressions.

Overall the code should be mode direct, this leads to better results in a few cases (fewer unnecessary LCSSA phis) and fixes the crash in the newly added test case.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134739

Files:
  llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
  llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
  llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll
  llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134739.463981.patch
Type: text/x-patch
Size: 10756 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220929/84120237/attachment.bin>