[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D76096#4564855 , @DavidSpickett 
wrote:

> One of the kernel tests now fails to build for AArch64 and Arm:
>
>   00:00:48 ++ make 
> CC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/bin/aarch64-cc 
> LD=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/ld.lld 
> SUBLEVEL=0 EXTRAVERSION=-bisect ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- 
> HOSTCC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/clang 
> -j32 -s -k
>   00:08:26 lib/test_scanf.c:661:2: error: implicit conversion from 'int' to 
> 'unsigned char' changes value from -168 to 88 [-Werror,-Wconstant-conversion]
>   00:08:26   661 | test_number_prefix(unsigned char,   "0xA7", 
> "%2hhx%hhx", 0, 0xa7, 2, check_uchar);
>   00:08:26   | 
> ^
>   00:08:26 lib/test_scanf.c:609:29: note: expanded from macro 
> 'test_number_prefix'
>   00:08:26   609 | T result[2] = {~expect[0], ~expect[1]};
>  \
>   00:08:26   |   ~^~
>   00:08:27 1 error generated.
>   00:08:27 make[3]: *** [scripts/Makefile.build:243: lib/test_scanf.o] Error 1
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/test_scanf.c#n661
>
> Seems like the test should be fixed, though I wonder why this same error 
> isn't effecting GCC builds.

Thanks for the report! I sent a patch for this already, just waiting for it to 
be picked up: 
https://lore.kernel.org/20230807-test_scanf-wconstant-conversion-v2-1-839ca3908...@kernel.org/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

One of the kernel tests now fails to build for AArch64 and Arm:

  00:00:48 ++ make 
CC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/bin/aarch64-cc 
LD=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/ld.lld 
SUBLEVEL=0 EXTRAVERSION=-bisect ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- 
HOSTCC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/clang 
-j32 -s -k
  00:08:26 lib/test_scanf.c:661:2: error: implicit conversion from 'int' to 
'unsigned char' changes value from -168 to 88 [-Werror,-Wconstant-conversion]
  00:08:26   661 | test_number_prefix(unsigned char,   "0xA7", 
"%2hhx%hhx", 0, 0xa7, 2, check_uchar);
  00:08:26   | 
^
  00:08:26 lib/test_scanf.c:609:29: note: expanded from macro 
'test_number_prefix'
  00:08:26   609 | T result[2] = {~expect[0], ~expect[1]};  
   \
  00:08:26   |   ~^~
  00:08:27 1 error generated.
  00:08:27 make[3]: *** [scripts/Makefile.build:243: lib/test_scanf.o] Error 1

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/test_scanf.c#n661

Seems like the test should be fixed, though I wonder why this same error isn't 
effecting GCC builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG610ec954e1f8: [clang] allow const structs/unions/arrays to 
be constant expressions for C (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/builtin-constant-p.c
  clang/test/CodeGen/const-init.c
  clang/test/Sema/builtins.c
  clang/test/Sema/init.c

Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -164,3 +164,46 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t) }; // expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar3 = {
+my_foo, // no-warning
+};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){
+my_foo // no-warning
+};
+extern const struct PR4517_foo my_foo2;
+struct PR4517_bar my_bar5 = {
+  .foo = my_foo2, // expected-error {{initializer element is not a compile-time constant}}
+};
+const struct PR4517_foo my_foo3 = {.x = my_foo.x};
+int PR4517_a[2] = {0, 1};
+const int PR4517_ca[2] = {0, 1};
+int PR4517_idx = 0;
+const int PR4517_idxc = 1;
+int PR4517_x1 = PR4517_a[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x2 = PR4517_a[PR4517_idxc]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x3 = PR4517_a[0]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y1 = PR4517_ca[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y2 = PR4517_ca[PR4517_idxc]; // no-warning
+int PR4517_y3 = PR4517_ca[0]; // no-warning
+union PR4517_u {
+int x;
+float y;
+};
+const union PR4517_u u1 = {4.0f};
+const union PR4517_u u2 = u1; // no-warning
+const union PR4517_u u3 = {u1.y}; // expected-error {{initializer element is not a compile-time constant}}
Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -131,7 +131,7 @@
 
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
-const char test17_d[] = {1, 2, 3, 4};
+const char test17_d[] = {1, 2, 3, 4}; // Like test17_c but not NUL-terminated.
 typedef int __attribute__((vector_size(16))) IntVector;
 struct Aggregate { int n; char c; };
 enum Enum { EnumValue1, EnumValue2 };
@@ -178,9 +178,10 @@
   ASSERT(!OPT("abcd"));
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
   // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folding}}
-  ASSERT(!OPT(_c[0]));// expected-warning {{folding}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folding}}
+  ASSERT(!OPT(test17_c));
+  ASSERT(!OPT(_c[0]));
+  ASSERT(!OPT((char*)test17_c));
+  // NOTE: test17_d is not NUL-termintated, so calling strlen on it is UB.
   ASSERT(!OPT(test17_d));// expected-warning {{folding}}
   ASSERT(!OPT(_d[0]));// expected-warning {{folding}}
   ASSERT(!OPT((char*)test17_d)); // expected-warning {{folding}}
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -190,3 +190,28 @@
 struct { const float *floats; } compoundliteral = {
   (float[1]) { 0.1, },
 };
+
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {.foo = my_foo};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){.foo = my_foo};
+struct PR4517_bar my_bar3 = {my_foo};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){my_foo};
+// CHECK: @my_foo = constant %struct.PR4517_foo { i32 42 }, align 4
+// CHECK: @my_bar = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar2 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar3 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar4 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+const int PR4517_arrc[2] = {41, 42};
+int PR4517_x = PR4517_arrc[1];
+const int PR4517_idx = 1;
+int PR4517_x2 = PR4517_arrc[PR4517_idx];
+// CHECK: @PR4517_arrc = constant [2 x i32] [i32 41, i32 

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

In D76096#4540442 , @nickdesaulniers 
wrote:

> Consider the following code:
>
>   long x [] = {1, 2, 3, 4, 5 + 4};
>
> Even with some of my recent changes, because the fast path parallel 
> implementation doesn't currently handle BinaryOperator Exprs, we'll visit all 
> of the ImplicitCastExpr and IntegerLiteral, then eventually figure out that 
> we can't constant evaluate the final element (for the wrong reasons; i.e. 
> VisitBinaryOperator not implemented).  That's a slow down because then we'll 
> fall back to EvaluateAsLValue/EvaluateAsRValue which will then succeed.
>
> So we pretty much need a fair amount of parallel implementation for the fast 
> path to succeed.  I'm happy to implement all that, if that's the direction 
> you're advising?
>
> Or did you mean something else when you said "for structs and arrays?"

The given example doesn't fallback in the "bad" way: we don't create an APValue 
representing `x[]`.  If you look at the code for lowering InitListExpr, you'll 
note that it doesn't actually just recursively Visit() like we do in other 
places.  We do create an lvalue for "5+4", but that isn't expensive in the same 
way.

> Is there another way I can prove the absence of regression to you?

At this point, I'm basically convinced the regressions are only going to be in 
obscure cases we aren't going to find by benchmarking.  I'm sure someone will 
find them eventually, but we can deal with them as they come up, I think.

So LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:74-76
+- ``structs``, ``unions``, and ``arrays`` that are const may now be used as
+  constant expressions.  This change is more consistent with the behavior of
+  GCC.

Note to self:
If this lands in time to be backported to release/17.x (as I've tagged 
https://github.com/llvm/llvm-project/issues/44502 to that milestone), I'll need 
to rebase this patch and post to a branch as the result of:

https://discourse.llvm.org/t/llvm-17-x-release-notes-update/72292


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4537078 , @efriedma wrote:

> The idea is to maintain the parallel infrastructure for structs and arrays, 
> but not for other things.

Are you referring specifically to InitListExprs on VarDecls of records 
(structs/arrays/unions)?

Consider the following code:

  long x [] = {1, 2, 3, 4, 5 + 4};

Even with some of my recent changes, because the fast path parallel 
implementation doesn't currently handle BinaryOperator Exprs, we'll visit all 
of the ImplicitCastExpr and IntegerLiteral, then eventually figure out that we 
can't constant evaluate the final element (for the wrong reasons; i.e. 
VisitBinaryOperator not implemented).  That's a slow down because then we'll 
fall back to EvaluateAsLValue/EvaluateAsRValue which will then succeed.

So we pretty much need a fair amount of parallel implementation for the fast 
path to succeed.  I'm happy to implement all that, if that's the direction 
you're advising?

Or did you mean something else when you said "for structs and arrays?"

> I'm trying to avoid regressions.

I think I've already shown  this patch 
to be an improvement for the Linux kernel build.

In D76096#4536264 , @nickdesaulniers 
wrote:

> Perhaps playing with time to compile programs generated via incbin 
>  would be a useful test, too?

Sorry, not incbin, xxd  is what I had 
in mind.

For another test, if I dump a 7kb image to a c file (image included for 
science):

F28472684: danger.jpeg 

  $ xxd --include ~/Pictures/danger.jpeg > danger.c
  $ head -n 3 danger.c
  unsigned char _usr_local_google_home_ndesaulniers_Pictures_danger_jpeg[] = {
0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, 0x49, 0x46, 0x00, 0x01,
0x01, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0xff, 0xdb, 0x00, 0x84,
  $ wc -l danger.c
  637 danger.c

With D76096 :

  $ hyperfine --runs 3000 'clang -c danger.c -o /dev/null'
  Benchmark 1: clang -c danger.c -o /dev/null
Time (mean ± σ):  26.2 ms ±   1.4 ms[User: 15.0 ms, System: 11.1 ms]
Range (min … max):22.1 ms …  36.6 ms3000 runs

baseline:

  $ hyperfine --runs 3000 'clang -c danger.c -o /dev/null'
  Benchmark 1: clang -c danger.c -o /dev/null
Time (mean ± σ):  27.1 ms ±   1.4 ms[User: 15.9 ms, System: 11.2 ms]
Range (min … max):23.4 ms …  36.1 ms3000 runs

Is there another way I can prove the absence of regression to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> But prior to D151587 , we did that for C++. 
> Why is C special here?  And prior to this patch, we did that for C++ 11+. Why 
> is C++ 03 special here?

I'm trying to avoid regressions.

C++11 made constant evaluation a lot more complicated, so in C++11 we 
evaluate() every global to determine const-ness.  But it's always worked that 
way, so there's no regression.  I'm not exactly happy C++11 made everything 
more expensive, but fixing that is a lot harder, and not a regression.

> So I feel like your feedback is pulling me in opposite directions. You want 
> to avoid the fast path falling back to the slow path, but improvements to the 
> fast path directly result in "complete parallel infrastructure" which you 
> also don't want. Those seem mutually exclusive to me. Is there a third 
> option? Am I attacking a strawman? (Regardless, I don't want to seem 
> unappreciative of the reviews, advice, or discussion).

The primary thing that makes Evaluate/APValue slow is the fact that it has a 
very inefficient representation of structs and arrays.  Using it to evaluate 
simple integer expressions is largely comparable to non-Evaluate() methods.  
The idea is to maintain the parallel infrastructure for structs and arrays, but 
not for other things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4533497 , @efriedma wrote:

> Basically, I don't want order-of-magnitude compile-time regressions with 
> large global variables.  There are basically two components to that:
>
> - That the fast path for emitting globals in C continues be fast.

https://reviews.llvm.org/D76096#4529555 had some numbers for the linux kernel.

Perhaps playing with time to compile programs generated via incbin 
 would be a useful test, too?

> - That we rarely end up using evaluateValue() on large global arrays/structs 
> in C, for reasons other than emitting them.  If we end up calling 
> evaluateValue() on most globals anyway, the codegen fast-path is a lot less 
> useful;

But prior to D151587 , we did that for C++. 
Why is C special here?

And prior to this patch, we did that for C++ 11+. Why is C++ 03 special here?

To find such cases where the CGExprConstant "fast path" is failing, I'm adding 
logging to the two cases from D151587  where 
the fast path fails but the slow path succeeds, then building the Linux kernel:

  diff --git a/clang/lib/CodeGen/CGExprConstant.cpp 
b/clang/lib/CodeGen/CGExprConstant.cpp
  index 9ad07f7d2220..6b618db281bd 100644
  --- a/clang/lib/CodeGen/CGExprConstant.cpp
  +++ b/clang/lib/CodeGen/CGExprConstant.cpp
  @@ -1677,8 +1683,14 @@ llvm::Constant 
*ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl ) {
   
 // Try to emit the initializer.  Note that this can allow some things that
 // are not allowed by tryEmitPrivateForMemory alone.
  -  if (APValue *value = D.evaluateValue())
  -return tryEmitPrivateForMemory(*value, destType);
  +  if (APValue *value = D.evaluateValue()) {
  +llvm::Constant *C = tryEmitPrivateForMemory(*value, destType);
  +if (C) {
  +  llvm::dbgs() << "ConstExprEmitted failed (but EvaluateValue 
succeeded):\n";
  +  E->dump();
  +}
  +return C;
  +  }
   
 return nullptr;
   }
  @@ -1760,8 +1772,14 @@ llvm::Constant *ConstantEmitter::tryEmitPrivate(const 
Expr *E,
 else
   Success = E->EvaluateAsRValue(Result, CGM.getContext(), 
InConstantContext);
   
  -  if (Success && !Result.HasSideEffects)
  -return tryEmitPrivate(Result.Val, destType);
  +  if (Success && !Result.HasSideEffects) {
  +llvm::Constant *C = tryEmitPrivate(Result.Val, destType);
  +if (C) {
  +  llvm::dbgs() << "ConstExprEmitter failed (but Evaluate succeeded):\n";
  +  E->dump();
  +}
  +return C;
  +  }
   
 return nullptr;
   }

There are still a lot of cases that could be improved in the fast path. 
DeclRefExpr seems error prone/more difficult, but there are still lots of 
stupid/simple cases (like `int x = -1;` being a UnaryOperator that currently 
isn't handled, and `long x = 1` having a widening ImplicitCast, `short x = 1` 
having a narrowing implicit cast, IIRC `sizeof` failed in the fast path even 
when not using DeclRefExpr).   But then when I go to improve the "fast path" 
you give me feedback that :

>> We can do a whole bunch of these, but at some point we hit diminishing 
>> returns... bailing out here isn't *that* expensive. Do you have some idea 
>> how far you want to go with this? Adding a couple obvious checks like this 
>> isn't a big deal, but I don't want to build up a complete parallel 
>> infrastructure for scalar expressions here.

So I feel like your feedback is pulling me in opposite directions.  You want to 
avoid the fast path falling back to the slow path, but improvements to the fast 
path directly result in "complete parallel infrastructure" which you also don't 
want.  Those seem mutually exclusive to me. Is there a third option?  Am I 
attacking a strawman? (Regardless, I don't want to seem unappreciative of the 
reviews, advice, or discussion).

> most of the cost of the APValue codepath is generating the APValue, not 
> lowering it.

That is my understanding of the "slow paths" as well.  I think I'll send a 
patch with just comments explaining this (which is really a follow up to 
D151587  more so than this patch).

> This is not quite as concrete as I'd like, but I'm not sure how to describe 
> it.

D151587  is in release/17.x and release/17.x 
has now branched. I'd like to try to get this patch cherry picked back to 
release/17.x, and as such I have marked 
https://github.com/llvm/llvm-project/issues/44502 as part of the release/17.x 
milestone.  Please let me know if you think that's too aggressive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Basically, I don't want order-of-magnitude compile-time regressions with large 
global variables.  There are basically two components to that:

- That the fast path for emitting globals in C continues be fast.
- That we rarely end up using evaluateValue() on large global arrays/structs in 
C, for reasons other than emitting them.  If we end up calling evaluateValue() 
on most globals anyway, the codegen fast-path is a lot less useful; most of the 
cost of the APValue codepath is generating the APValue, not lowering it.

This is not quite as concrete as I'd like, but I'm not sure how to describe it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 544128.
nickdesaulniers added a comment.

- rebase for presubmit builders


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/builtin-constant-p.c
  clang/test/CodeGen/const-init.c
  clang/test/Sema/builtins.c
  clang/test/Sema/init.c

Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -164,3 +164,46 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t) }; // expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar3 = {
+my_foo, // no-warning
+};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){
+my_foo // no-warning
+};
+extern const struct PR4517_foo my_foo2;
+struct PR4517_bar my_bar5 = {
+  .foo = my_foo2, // expected-error {{initializer element is not a compile-time constant}}
+};
+const struct PR4517_foo my_foo3 = {.x = my_foo.x};
+int PR4517_a[2] = {0, 1};
+const int PR4517_ca[2] = {0, 1};
+int PR4517_idx = 0;
+const int PR4517_idxc = 1;
+int PR4517_x1 = PR4517_a[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x2 = PR4517_a[PR4517_idxc]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x3 = PR4517_a[0]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y1 = PR4517_ca[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y2 = PR4517_ca[PR4517_idxc]; // no-warning
+int PR4517_y3 = PR4517_ca[0]; // no-warning
+union PR4517_u {
+int x;
+float y;
+};
+const union PR4517_u u1 = {4.0f};
+const union PR4517_u u2 = u1; // no-warning
+const union PR4517_u u3 = {u1.y}; // expected-error {{initializer element is not a compile-time constant}}
Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -131,7 +131,7 @@
 
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
-const char test17_d[] = {1, 2, 3, 4};
+const char test17_d[] = {1, 2, 3, 4}; // Like test17_c but not NUL-terminated.
 typedef int __attribute__((vector_size(16))) IntVector;
 struct Aggregate { int n; char c; };
 enum Enum { EnumValue1, EnumValue2 };
@@ -178,9 +178,10 @@
   ASSERT(!OPT("abcd"));
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
   // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folding}}
-  ASSERT(!OPT(_c[0]));// expected-warning {{folding}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folding}}
+  ASSERT(!OPT(test17_c));
+  ASSERT(!OPT(_c[0]));
+  ASSERT(!OPT((char*)test17_c));
+  // NOTE: test17_d is not NUL-termintated, so calling strlen on it is UB.
   ASSERT(!OPT(test17_d));// expected-warning {{folding}}
   ASSERT(!OPT(_d[0]));// expected-warning {{folding}}
   ASSERT(!OPT((char*)test17_d)); // expected-warning {{folding}}
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -190,3 +190,28 @@
 struct { const float *floats; } compoundliteral = {
   (float[1]) { 0.1, },
 };
+
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {.foo = my_foo};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){.foo = my_foo};
+struct PR4517_bar my_bar3 = {my_foo};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){my_foo};
+// CHECK: @my_foo = constant %struct.PR4517_foo { i32 42 }, align 4
+// CHECK: @my_bar = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar2 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar3 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar4 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+const int PR4517_arrc[2] = {41, 42};
+int PR4517_x = PR4517_arrc[1];
+const int PR4517_idx = 1;
+int PR4517_x2 = PR4517_arrc[PR4517_idx];
+// CHECK: @PR4517_arrc = constant [2 x i32] [i32 41, i32 42], align 4
+// CHECK: @PR4517_x = global i32 42, align 4
+// CHECK: @PR4517_idx = constant i32 1, align 4
+// CHECK: @PR4517_x2 = global i32 42, align 

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

what's the "exit criteria" for this patch (https://reviews.llvm.org/D76096)? in 
https://reviews.llvm.org/D76096#4523828 and 
https://reviews.llvm.org/D76096#4524003 you mention InitListExpr and 
StringLiteral. In https://reviews.llvm.org/D156185#4533274 I verified that 
StringLiteral is handled by the fast path. I believe that InitListExpr is as 
well.  Is your (@efriedma ) concern more so about large InitListExpr and large 
StringLiteral (more so than outright support)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Posting quick benchmarks of linux kernel builds:

ac524886094db58112ca176e1d727330a94634a8 
 + D151587 
 + D76096  + 
D156154 :

  $ hyperfine --prepare 'make LLVM=1 -j128 -s clean' --runs 30 'make LLVM=1 
-j128 -s'
  Benchmark 1: make LLVM=1 -j128 -s
Time (mean ± σ): 65.887 s ±  0.592 s[User: 4953.517 s, System: 
278.530 s]
Range (min … max):   65.008 s … 68.578 s30 runs

ac524886094db58112ca176e1d727330a94634a8 
 (i.e. 
baseline):

  $ hyperfine --prepare 'make LLVM=1 -j128 -s clean' --runs 30 'make LLVM=1 
-j128 -s'
  Benchmark 1: make LLVM=1 -j128 -s
Time (mean ± σ): 67.140 s ±  0.201 s[User: 5079.011 s, System: 
291.879 s]
Range (min … max):   66.764 s … 67.579 s30 runs

---

So it looks like this series speeds up the mean by ~1.9%.  So to partially and 
proactively answer the question "does this series result in a performance 
regression?" at least for the linux kernel, it seems like no. But YMMV per 
codebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Is the idea for the way forward here to ensure (i.e. adding code such) that 
> ConstExprEmitter can constant evaluate such Expr's?

For that exact construct, EvaluateAsRValue will also fail, so there's no real 
regression.  The issue would be for a constant global variable... but if we try 
to handle that, we get into the exact mess of complicated code for 
lvalue-to-rvalue conversions in CGExprConstant we were trying to avoid adding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Hmm, that kind of construct could run into issues with large global variables.  
If we "inline" a bunch of large global constants into initializers for arrays, 
we could significantly increase codesize.  Not sure how likely that is in 
practice.  We could maybe consider teaching CGExprConstant to abort in such 
cases (not sure if it currently tracks whether we're initializing a global 
variable or a local.)  Not sure what the heuristics for that would look like.

Thinking about it a bit more, maybe we should just observe overall compiler 
behavior.  If the compile-time and codesize for the Linux kernel (and maybe 
also the llvm test-suite) is mostly unchanged, we're probably fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4524092 , @nickdesaulniers 
wrote:

> In D76096#4524003 , @efriedma wrote:
>
>> The ones most likely to be a concern are InitListExpr and StringLiteral.
>
> Here's a reduced instance from the Linux kernel:
>
>   struct timespec64 {
> long tv_sec;
> long tv_nsec;
>   } do_utime_mtime;
>   void __attribute__do_utime() { struct timespec64 t[] = {{}, 
> do_utime_mtime}; }
>
> Trace: https://paste.debian.net/1286590/

IIUC what's going wrong here, I think that ConstExprEmitter is //not// able to 
evaluate the InitListExpr, and is bailing/falling back to call 
Expr::EvaluateAsRValue().

Is the idea for the way forward here to ensure (i.e. adding code such) that 
ConstExprEmitter //can// constant evaluate such Expr's?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4524003 , @efriedma wrote:

> The ones most likely to be a concern are InitListExpr and StringLiteral.

Here's a reduced instance from the Linux kernel:

  struct timespec64 {
long tv_sec;
long tv_nsec;
  } do_utime_mtime;
  void __attribute__do_utime() { struct timespec64 t[] = {{}, do_utime_mtime}; }

Trace: https://paste.debian.net/1286590/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The ones most likely to be a concern are InitListExpr and StringLiteral.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4523828 , @efriedma wrote:

> The idea would be looking for places we EvaluateAsRValue an array or struct.  
> Not sure what that stack trace represents.

Perhaps you mean calling `EvaluateAsRValue` with a `InitListExpr`?  I assume 
your concern is evaluating the (potentially large) init list?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The idea would be looking for places we EvaluateAsRValue an array or struct.  
Not sure what that stack trace represents.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4523750 , @nickdesaulniers 
wrote:

> In D76096#4523718 , @efriedma wrote:
>
>> My primary concern here is making sure we don't actually blow up 
>> compile-time.   D151587  fixes the 
>> dependency from CGExprConstant, which was the most complicated one, but 
>> there are other places that call into Evaluate().  Some of those are 
>> probably unavoidable, but I'd like to make sure at least that we don't end 
>> up evaluating the initializers of global variables in C code.
>>
>> Not sure there's any good way to regression-test that, though...
>
> I could probably add a call to abort() somewhere and then build the Linux 
> kernel then verify that we don't abort.  Which `Evaluate` method of which 
> class should I try to add that to?

oh yeah, that blew up pretty fast. The trace: https://paste.debian.net/1286583/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 543051.
nickdesaulniers added a comment.

- use better identifiers in test
- remove half baked comment from test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/builtin-constant-p.c
  clang/test/CodeGen/const-init.c
  clang/test/Sema/builtins.c
  clang/test/Sema/init.c

Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -164,3 +164,46 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t) }; // expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar3 = {
+my_foo, // no-warning
+};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){
+my_foo // no-warning
+};
+extern const struct PR4517_foo my_foo2;
+struct PR4517_bar my_bar5 = {
+  .foo = my_foo2, // expected-error {{initializer element is not a compile-time constant}}
+};
+const struct PR4517_foo my_foo3 = {.x = my_foo.x};
+int PR4517_a[2] = {0, 1};
+const int PR4517_ca[2] = {0, 1};
+int PR4517_idx = 0;
+const int PR4517_idxc = 1;
+int PR4517_x1 = PR4517_a[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x2 = PR4517_a[PR4517_idxc]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x3 = PR4517_a[0]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y1 = PR4517_ca[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y2 = PR4517_ca[PR4517_idxc]; // no-warning
+int PR4517_y3 = PR4517_ca[0]; // no-warning
+union PR4517_u {
+int x;
+float y;
+};
+const union PR4517_u u1 = {4.0f};
+const union PR4517_u u2 = u1; // no-warning
+const union PR4517_u u3 = {u1.y}; // expected-error {{initializer element is not a compile-time constant}}
Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -131,7 +131,7 @@
 
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
-const char test17_d[] = {1, 2, 3, 4};
+const char test17_d[] = {1, 2, 3, 4}; // Like test17_c but not NUL-terminated.
 typedef int __attribute__((vector_size(16))) IntVector;
 struct Aggregate { int n; char c; };
 enum Enum { EnumValue1, EnumValue2 };
@@ -178,9 +178,10 @@
   ASSERT(!OPT("abcd"));
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
   // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folding}}
-  ASSERT(!OPT(_c[0]));// expected-warning {{folding}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folding}}
+  ASSERT(!OPT(test17_c));
+  ASSERT(!OPT(_c[0]));
+  ASSERT(!OPT((char*)test17_c));
+  // NOTE: test17_d is not NUL-termintated, so calling strlen on it is UB.
   ASSERT(!OPT(test17_d));// expected-warning {{folding}}
   ASSERT(!OPT(_d[0]));// expected-warning {{folding}}
   ASSERT(!OPT((char*)test17_d)); // expected-warning {{folding}}
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -190,3 +190,28 @@
 struct { const float *floats; } compoundliteral = {
   (float[1]) { 0.1, },
 };
+
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {.foo = my_foo};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){.foo = my_foo};
+struct PR4517_bar my_bar3 = {my_foo};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){my_foo};
+// CHECK: @my_foo = constant %struct.PR4517_foo { i32 42 }, align 4
+// CHECK: @my_bar = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar2 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar3 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar4 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+const int PR4517_arrc[2] = {41, 42};
+int PR4517_x = PR4517_arrc[1];
+const int PR4517_idx = 1;
+int PR4517_x2 = PR4517_arrc[PR4517_idx];
+// CHECK: @PR4517_arrc = constant [2 x i32] [i32 41, i32 42], align 4
+// CHECK: @PR4517_x = global i32 42, align 4
+// CHECK: @PR4517_idx = constant i32 1, align 4
+// 

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#4523718 , @efriedma wrote:

> My primary concern here is making sure we don't actually blow up 
> compile-time.   D151587  fixes the 
> dependency from CGExprConstant, which was the most complicated one, but there 
> are other places that call into Evaluate().  Some of those are probably 
> unavoidable, but I'd like to make sure at least that we don't end up 
> evaluating the initializers of global variables in C code.
>
> Not sure there's any good way to regression-test that, though...

I could probably add a call to abort() somewhere and then build the Linux 
kernel then verify that we don't abort.  Which `Evaluate` method of which class 
should I try to add that to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 543048.
nickdesaulniers retitled this revision from "[clang] allow const structs to be 
constant expressions for C" to "[clang] allow const structs/unions/arrays to be 
constant expressions for C".
nickdesaulniers added a comment.

- add moar tests for unions
- modify commit one line description to mentions arrays and unions, since those 
are also records


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/builtin-constant-p.c
  clang/test/CodeGen/const-init.c
  clang/test/Sema/builtins.c
  clang/test/Sema/init.c

Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -164,3 +164,48 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t) }; // expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){
+.foo = my_foo, // no-warning
+};
+struct PR4517_bar my_bar3 = {
+my_foo, // no-warning
+};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){
+my_foo // no-warning
+};
+extern const struct PR4517_foo my_foo2;
+struct PR4517_bar my_bar5 = {
+  .foo = my_foo2, // expected-error {{initializer element is not a compile-time constant}}
+};
+const struct PR4517_foo my_foo3 = {.x = my_foo.x};
+int PR4517_a[2] = {0, 1};
+const int PR4517_ca[2] = {0, 1};
+int PR4517_idx = 0;
+const int PR4517_idxc = 1;
+int PR4517_x1 = PR4517_a[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x2 = PR4517_a[PR4517_idxc]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_x3 = PR4517_a[0]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y1 = PR4517_ca[PR4517_idx]; // expected-error {{initializer element is not a compile-time constant}}
+int PR4517_y2 = PR4517_ca[PR4517_idxc]; // no-warning
+int PR4517_y3 = PR4517_ca[0]; // no-warning
+
+// Unions are still
+union hello {
+int x;
+float y;
+};
+const union hello my_hello = {4.0f};
+const union hello my_hello2 = my_hello; // no-warning
+const union hello my_hello3 = {my_hello.y}; // expected-error {{initializer element is not a compile-time constant}}
Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -131,7 +131,7 @@
 
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
-const char test17_d[] = {1, 2, 3, 4};
+const char test17_d[] = {1, 2, 3, 4}; // Like test17_c but not NUL-terminated.
 typedef int __attribute__((vector_size(16))) IntVector;
 struct Aggregate { int n; char c; };
 enum Enum { EnumValue1, EnumValue2 };
@@ -178,9 +178,10 @@
   ASSERT(!OPT("abcd"));
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
   // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folding}}
-  ASSERT(!OPT(_c[0]));// expected-warning {{folding}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folding}}
+  ASSERT(!OPT(test17_c));
+  ASSERT(!OPT(_c[0]));
+  ASSERT(!OPT((char*)test17_c));
+  // NOTE: test17_d is not NUL-termintated, so calling strlen on it is UB.
   ASSERT(!OPT(test17_d));// expected-warning {{folding}}
   ASSERT(!OPT(_d[0]));// expected-warning {{folding}}
   ASSERT(!OPT((char*)test17_d)); // expected-warning {{folding}}
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -190,3 +190,28 @@
 struct { const float *floats; } compoundliteral = {
   (float[1]) { 0.1, },
 };
+
+struct PR4517_foo {
+  int x;
+};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {.x = 42};
+struct PR4517_bar my_bar = {.foo = my_foo};
+struct PR4517_bar my_bar2 = (struct PR4517_bar){.foo = my_foo};
+struct PR4517_bar my_bar3 = {my_foo};
+struct PR4517_bar my_bar4 = (struct PR4517_bar){my_foo};
+// CHECK: @my_foo = constant %struct.PR4517_foo { i32 42 }, align 4
+// CHECK: @my_bar = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar2 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar3 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+// CHECK: @my_bar4 = global %struct.PR4517_bar { %struct.PR4517_foo { i32 42 } }, align 4
+const int