[PATCH] D144316: [SCEV] Fix FoldID::addInteger(unsigned long I)

Sat Feb 18 13:19:05 PST 2023

vitalybuka added a comment.

As is it's a really land mine, it's cut off half of the pointer in the key, and can affect correctness anything that uses SCEV

Regarding perf, I have improved patch, which I planed to send for review, but not to backport:
https://llvm-compile-time-tracker.com/index.php?config=NewPM-O3&stat=instructions%3Au&remote=vitalybuka

We can backport both of them, to be incremental:

1. correctness
2. perf

In D144316#4137232 <https://reviews.llvm.org/D144316#4137232>, @nikic wrote:

> Looks like this had a fairly significant compile-time impact: https://llvm-compile-time-tracker.com/compare.php?from=c23f29d6f05b1fe4fa2dd50cbb78ee2b30e0de4d&to=a53d940cee6f281ef1a20d4f0fb39b23b4e98614&stat=instructions:u
>
> I think the current uses of FoldID will now have 5 elements and as such overflow SmallVector space, each resulting in a vector allocation?
>
> It's not great that this change was backported without approval, bypassing the required backport process, immediately after landing the change on main, without even waiting for any fallout :(

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144316