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

Tue Jul 25 09:10:34 PDT 2023

aaron.ballman added a comment.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155955