[PATCH] D155955: [Clang] Improve the handling of large arrays evaluation.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D155955#4532433 <https://reviews.llvm.org/D155955#4532433>, @cor3ntin wrote:

> In D155955#4532385 <https://reviews.llvm.org/D155955#4532385>, @aaron.ballman wrote:
>
>> In D155955#4528792 <https://reviews.llvm.org/D155955#4528792>, @efriedma wrote:
>>
>>>> Note sure what you mean. Making sure we use size_t for all array extents is not something that can be fixed overnight but more importantly, it does not help: Would it not overflow, the allocation would still fail.
>>>
>>> Oh, right, it would be sizeof(APValue) * UINT_MAX, which would break in any practical usage.
>>>
>>>> I don't think that would make sense in actual code, and having some sort of sparse array is something I considered. And we already delay allocation in some cases, but we do need to create elements to destroy them, read them, etc. So while it may be possible, it seems... complicated.
>>>
>>> Definitely not simple, sure.
>>>
>>>> I think a more viable long term fix might be to not crash on allocation failure, and/or to have a way to limit the allocation to some portion of the available memory.
>>>
>>> Making the compiler's behavior depend on the amount of memory installed in the user's computer seems like a non-starter.  I think we just have to stick with some combination of:
>>>
>>> - Try to reduce excessive memory usage in constant folding.
>>
>> 100% agreed
>>
>>> - Add strict limits memory usage limits for optional constant folding
>>
>> This seems reasonable to me, but I think this should be user controllable so that users with different resource constraints can override whatever default value we pick.
>>
>>> - Maybe consider disobeying the standard slightly in certain cases: the standard requires that we constant-fold the initializers for all global variables, but that might not really be viable for globals that are expensive to evaluate.
>>
>> If we can use an existing implementation limit (http://eel.is/c++draft/implimits) to justify our decision, great; otherwise, I think we should ask WG21 for an official escape hatch so that we don't have conformance issues.
>
> Forgot to reply to that, it's already conforming

Oh, excellent!

> - A variable or temporary object o is constant-initialized if [..] the full-expression of its initialization is a constant expression http://eel.is/c++draft/expr.const#2
> - An expression E is a core constant expression unless [...] it would exceed the implementation-defined limits http://eel.is/c++draft/expr.const#5
> - Limits may constrain quantities that include those described below or others. http://eel.is/c++draft/implimits#2.sentence-1
>
> On the last point, i think a *explicit* limit would be nice but i chatted with core and they agreed the current wording was sufficient - and they didn't seem super enthusiastic about adding limits.

"or others" is an amazing amount of latitude for Core to give us, how handy! :-D

FWIW, I personally think having explicit limits is far better than relying on "or others" to do the heavy lifting, but we've got what we need at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155955