[PATCH] D137505: [SCEV] Cache ZExt SCEV expressions.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D137505#3921762 <https://reviews.llvm.org/D137505#3921762>, @fhahn wrote:

> In D137505#3910603 <https://reviews.llvm.org/D137505#3910603>, @nikic wrote:
>
>> To make sure I understand the idea here correctly: We already cache the case where the zext does //not// fold through the early folding set lookup. This additional cache only helps with the case where the zext //does// fold, and the early lookup thus fails, is that correct?
>>
>> If so, I feel like this needs some more general solution. We're using the same pattern (early folding set lookup to bypass most of the folding logic) in most (all?) other SCEV creation methods as well, and introducing an independent cache for each of these would not be great. I wonder if we could insert a "forwarding" node into the folding set for all folded SCEV expressions, which points to the folding result, or something along those lines.
>
> Yes, that's exactly the issue. Currently the folding set (UniqueSCEV) only contains ZExt entries if we actually created a ZExt expression, not if it got folded to a different expression.
>
> Adding forwarding nodes so we can add references to folded expressions would work I think, but it might be easier to have maintain a separate map for folds with a more general key. I updated the patch to use a more flexible key (similar to FoldingSetNodeID, but with a smaller bits vector) to sketch this out. The new version is mostly neutral in compile-time on CTMark, but still has the same huge improvements on the cases mentioned in the description. (https://llvm-compile-time-tracker.com/compare.php?from=2116d69f100c243069be1e76ac7fdac65ea5328a&to=05897510fbf52a59e0ce28f3f4ea4d06d59e380e&stat=instructions:u)
>
> If we go down the forwarding route, IIUC we would have the add a new SCEV type for that. Happy to play around with that option as well, if people feel this would be more desirable.

My main thought here was that a forwarding node would avoid the need to do two folding set lookups, and should also avoid the need for the separate FoldID.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137505