[PATCH] D113578: [SCEV] Apply loop guards when computing max BTC for arbitrary steps.

Wed Dec 15 10:07:13 PST 2021

alexfh added a comment.

In D113578#3195175 <https://reviews.llvm.org/D113578#3195175>, @alexfh wrote:

> In D113578#3195059 <https://reviews.llvm.org/D113578#3195059>, @fhahn wrote:
>
>> In D113578#3195050 <https://reviews.llvm.org/D113578#3195050>, @alexfh wrote:
>>
>>> In D113578#3194811 <https://reviews.llvm.org/D113578#3194811>, @fhahn wrote:
>>>
>>>> I took a closer look and confirmed that the patch indeed slightly increases the call stack for the reproducer and this causes the stack overflow.
>>>>
>>>> The issue is roughly the following: we apply the loop guards to the last loop in a large chain of loops. The loop guard itself is the exit condition of an earlier loop. To construct the SCEV, we compute the exit count along one of the code paths. This in turn applies loop guards  which is in an earlier predecessor loop. To construct the SCEV, we again need to compute the exit count of this loop, applying loop guards and so on.
>>>>
>>>> This can be fixed by adjusting the order we apply loop guards. At the moment, the guard closest to the starting loop is evaluated first, triggering the large evaluation chain. If we instead apply the earliest guard first no excessive call chain is needed. This seems to have a tiny positive compile-time impact http://llvm-compile-time-tracker.com/compare.php?from=529833377ccdf4381f8bc9961bfa96ec4f5e2eed&to=b07452a70ac22ae4b1f0dbf4b84df3ee44c171a1&stat=instructions
>>>>
>>>> It is not quite NFC, because the order in which guards are applied can impact the result in some cases. I looked into some and so far it looks like this new order makes applying loop guards slightly more effective. I am planning on isolating a test that shows the improved analysis results and commit the fix soonish.
>>>
>>> Thanks! I guess, I can come up a couple of distinct test cases that have the same effect (compiler stack overflow). Would you like to have a look at them as well?
>>
>> The fix is available here: https://github.com/llvm/llvm-project/commit/b07452a70ac22ae4b1f0dbf4b84df3ee44c171a1, in case you want to give it a try against the other test cases.
>
> The patch fixes the crash, but it looks like the compilation time with the patch grows by a factor of ~7 for one of the problematic files. I'll try to make a cleaner experiment with two compilers built with exactly the same compiler options, but I doubt it will change much.

Scratch that. It wasn't a valid experiment at all.  Looking further.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113578