Page MenuHomePhabricator

use zeroinitializer for (trailing zero portion of) large array initializers more reliably
ClosedPublic

Authored by rsmith on May 21 2018, 3:49 PM.

Details

Summary

Clang has two different ways it emits array constants (from InitListExprs and from APValues), and both had some ability to emit zeroinitializer, but neither was able to catch all cases where we could use zeroinitializer reliably. In particular, emitting from an APValue would fail to notice if all the explicit array elements happened to be zero. In addition, for large arrays where only an initial portion has an explicit initializer, we would emit the complete initializer (which could be huge) rather than emitting only the non-zero portion. With this change, when the element would have a suffix of more than 8 zero elements, we emit the array constant as a packed struct of its initial portion followed by a zeroinitializer constant for the trailing zero portion.

In passing, I found a bug where SemaInit would sometimes walk the entire array when checking an initializer that only covers the first few elements; that's fixed here to unblock testing of the rest.

Diff Detail

Repository
rC Clang

Event Timeline

rsmith created this revision.May 21 2018, 3:49 PM
sepavloff accepted this revision.May 22 2018, 10:34 AM

Now I understand your idea about moving the logic to tryEmitPrivateForMemory. Indeed it is more general and consistent solution.
Thank you.

lib/CodeGen/CGExprConstant.cpp
643

s/Figre/Figure/ ?

903

The check for fillC != nullptr here is redundant, it was checked few lines above.

test/CodeGenCXX/cxx11-initializer-aggregate.cpp
83

Array definitions:

char data_8[1000 * 1000 * 1000] = {};
int (&&data_9)[1000 * 1000 * 1000] = {0};

also compile successfully with this patch and hang compiler without it.

This revision is now accepted and ready to land.May 22 2018, 10:34 AM

I like this approach a lot.

lib/CodeGen/CGExprConstant.cpp
675

Why std::vector?

rsmith updated this revision to Diff 148079.May 22 2018, 12:53 PM
rsmith marked 4 inline comments as done.
rsmith added inline comments.
lib/CodeGen/CGExprConstant.cpp
675

Only because this was extracted from the old version of the code. Switched to a SmallVector.

test/CodeGenCXX/cxx11-initializer-aggregate.cpp
83

I added these test cases, thanks!

rjmccall accepted this revision.May 22 2018, 1:05 PM

LGTM, thanks!

This revision was automatically updated to reflect the committed changes.