[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> It makes sense to me that const int foo[] = [ ...massive list...]; would take 
> long to validate the entire initializer as all constant expressions

The expensive part we're currently avoiding by bailing out of the constant 
evaluator (the code D76169  removes) is 
actually constructing an APValue; constructing APValues for large arrays and 
structs is disproportionately expensive compared to just walking the 
corresponding AST.




Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

nickdesaulniers wrote:
> efriedma wrote:
> > rsmith wrote:
> > > efriedma wrote:
> > > > rsmith wrote:
> > > > > nickdesaulniers wrote:
> > > > > > efriedma wrote:
> > > > > > > You need to be more careful here; we can call ConstExprEmitter 
> > > > > > > for arbitrary expressions.
> > > > > > "Be more careful" how?
> > > > > Here are some specific cases in which you need to be more careful 
> > > > > because the code as-is would do the wrong thing:
> > > > > 
> > > > >  * when emitting a global's initializer in C++, where the value of 
> > > > > the object denoted by the DeclRefExpr could have changed between its 
> > > > > initialization and the expression we're currently emitting 
> > > > >  * when emitting anything other than a global initializer in C, where 
> > > > > the value of a global could have changed after its emission
> > > > >  * when emitting a reference to a non-global declaration in C (local 
> > > > > variables might change between initialization and use)
> > > > > 
> > > > > We would need to restrict this to the cases where the variable's 
> > > > > value cannot possibly have changed between initialization and use.
> > > > > 
> > > > > In C, that's (mostly) the case for a static storage variable 
> > > > > referenced from the initializer of a static storage variable, for a 
> > > > > thread storage variable referenced from the initializer of a static 
> > > > > storage variable, or for a thread storage variable referenced from 
> > > > > the initializer of a thread storage variable. Even then, this isn't 
> > > > > strictly correct in the presence of DSOs, but I think it should be 
> > > > > correct if the two variables are defined in the same translation unit.
> > > > > 
> > > > > In C++, that's (mostly) the case when the variable is `const` or 
> > > > > `constexpr` and has no mutable subobjects. (There's still the case 
> > > > > where the reference happens outside the lifetime of the object -- for 
> > > > > the most part we can handwave that away by saying it must be UB, but 
> > > > > that's not true in general in the period of construction and period 
> > > > > of destruction.)
> > > > > 
> > > > > In both cases, the optimization is (arguably) still wrong if there 
> > > > > are any volatile subobjects.
> > > > And this is why I don't want to duplicate the logic. :)
> > > > 
> > > > I'd rather not make different assumptions for C and C++; instead, I'd 
> > > > prefer to just use the intersection that's safe in both.  I'm concerned 
> > > > that we could come up with weird results for mixed C and C++ code, 
> > > > otherwise.  Also, it's easier to define the C++ rules because we can 
> > > > base them on the constexpr rules in the standard.
> > > I agree we probably want the same outcome as D76169 provides, if either 
> > > the performance is acceptable or we can find a way to avoid the 
> > > performance cost for the cases we already accept. Perhaps we could get 
> > > that outcome by ensuring that we try the CGExprConstant fast-path (at 
> > > least in C compilations, maybe in general) before we consider the 
> > > complete-but-slow evaluation strategy in ExprConstant?
> > I like the idea of guarding constant evaluation with a "fast path" that 
> > doesn't actually compute the APValues in simple cases.  We can just 
> > implement the simple stuff, and fall back to the full logic for complicated 
> > stuff.
> > 
> > My one concern is that we could go to a bunch of effort to emit a variable 
> > on the fast path, then fall off the fast path later because "return a[0];" 
> > tries to constant-fold a big array "a".
> > the CGExprConstant fast-path
> 
> Sorry, what is "the CGExprConstant fast-path"?
The "fast-path" would be the existing code in ConstExprEmitter.   The idea is 
to make the constant evaluator the primary source of truth in both C and C++, 
along the lines of D76169, but keep some code around in CGExprConstant to go 
directly from the AST to LLVM IR in cases where we don't really need the full 
power of the constant evaluator.

So take the following steps:
- Change 
ConstantEmitter::tryEmitPrivate/ConstantEmitter::tryEmitPrivateForVarInit to 
try ConstExprEmitter first, instead of falling back to ConstExprEmitter.
- Fix any bugs that come out of that.
- Change the constant evaluator to handle struct

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

So, @rsmith are you ok with a patch like https://reviews.llvm.org/D76169 that 
removes those fixmes?

It makes sense to me that `const int foo[] = [ ...massive list...];` would take 
long to validate the entire initializer as all constant expressions, but I'm 
simply trying to allow:

  const int foo[] = [ ...massive list...];
  int bar = foo[0];




Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

efriedma wrote:
> rsmith wrote:
> > efriedma wrote:
> > > rsmith wrote:
> > > > nickdesaulniers wrote:
> > > > > efriedma wrote:
> > > > > > You need to be more careful here; we can call ConstExprEmitter for 
> > > > > > arbitrary expressions.
> > > > > "Be more careful" how?
> > > > Here are some specific cases in which you need to be more careful 
> > > > because the code as-is would do the wrong thing:
> > > > 
> > > >  * when emitting a global's initializer in C++, where the value of the 
> > > > object denoted by the DeclRefExpr could have changed between its 
> > > > initialization and the expression we're currently emitting 
> > > >  * when emitting anything other than a global initializer in C, where 
> > > > the value of a global could have changed after its emission
> > > >  * when emitting a reference to a non-global declaration in C (local 
> > > > variables might change between initialization and use)
> > > > 
> > > > We would need to restrict this to the cases where the variable's value 
> > > > cannot possibly have changed between initialization and use.
> > > > 
> > > > In C, that's (mostly) the case for a static storage variable referenced 
> > > > from the initializer of a static storage variable, for a thread storage 
> > > > variable referenced from the initializer of a static storage variable, 
> > > > or for a thread storage variable referenced from the initializer of a 
> > > > thread storage variable. Even then, this isn't strictly correct in the 
> > > > presence of DSOs, but I think it should be correct if the two variables 
> > > > are defined in the same translation unit.
> > > > 
> > > > In C++, that's (mostly) the case when the variable is `const` or 
> > > > `constexpr` and has no mutable subobjects. (There's still the case 
> > > > where the reference happens outside the lifetime of the object -- for 
> > > > the most part we can handwave that away by saying it must be UB, but 
> > > > that's not true in general in the period of construction and period of 
> > > > destruction.)
> > > > 
> > > > In both cases, the optimization is (arguably) still wrong if there are 
> > > > any volatile subobjects.
> > > And this is why I don't want to duplicate the logic. :)
> > > 
> > > I'd rather not make different assumptions for C and C++; instead, I'd 
> > > prefer to just use the intersection that's safe in both.  I'm concerned 
> > > that we could come up with weird results for mixed C and C++ code, 
> > > otherwise.  Also, it's easier to define the C++ rules because we can base 
> > > them on the constexpr rules in the standard.
> > I agree we probably want the same outcome as D76169 provides, if either the 
> > performance is acceptable or we can find a way to avoid the performance 
> > cost for the cases we already accept. Perhaps we could get that outcome by 
> > ensuring that we try the CGExprConstant fast-path (at least in C 
> > compilations, maybe in general) before we consider the complete-but-slow 
> > evaluation strategy in ExprConstant?
> I like the idea of guarding constant evaluation with a "fast path" that 
> doesn't actually compute the APValues in simple cases.  We can just implement 
> the simple stuff, and fall back to the full logic for complicated stuff.
> 
> My one concern is that we could go to a bunch of effort to emit a variable on 
> the fast path, then fall off the fast path later because "return a[0];" tries 
> to constant-fold a big array "a".
> the CGExprConstant fast-path

Sorry, what is "the CGExprConstant fast-path"?


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 to be constant expressions in initializer lists

[Tom Hughes via Phabricator via cfe-commits]

tomhughes added a comment.

I'm hitting this in the codebase that I'm working on as well. What are the next 
steps (besides reformatting)? It's not completely clear to me from the 
comments. Thanks!


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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

rsmith wrote:
> efriedma wrote:
> > rsmith wrote:
> > > nickdesaulniers wrote:
> > > > efriedma wrote:
> > > > > You need to be more careful here; we can call ConstExprEmitter for 
> > > > > arbitrary expressions.
> > > > "Be more careful" how?
> > > Here are some specific cases in which you need to be more careful because 
> > > the code as-is would do the wrong thing:
> > > 
> > >  * when emitting a global's initializer in C++, where the value of the 
> > > object denoted by the DeclRefExpr could have changed between its 
> > > initialization and the expression we're currently emitting 
> > >  * when emitting anything other than a global initializer in C, where the 
> > > value of a global could have changed after its emission
> > >  * when emitting a reference to a non-global declaration in C (local 
> > > variables might change between initialization and use)
> > > 
> > > We would need to restrict this to the cases where the variable's value 
> > > cannot possibly have changed between initialization and use.
> > > 
> > > In C, that's (mostly) the case for a static storage variable referenced 
> > > from the initializer of a static storage variable, for a thread storage 
> > > variable referenced from the initializer of a static storage variable, or 
> > > for a thread storage variable referenced from the initializer of a thread 
> > > storage variable. Even then, this isn't strictly correct in the presence 
> > > of DSOs, but I think it should be correct if the two variables are 
> > > defined in the same translation unit.
> > > 
> > > In C++, that's (mostly) the case when the variable is `const` or 
> > > `constexpr` and has no mutable subobjects. (There's still the case where 
> > > the reference happens outside the lifetime of the object -- for the most 
> > > part we can handwave that away by saying it must be UB, but that's not 
> > > true in general in the period of construction and period of destruction.)
> > > 
> > > In both cases, the optimization is (arguably) still wrong if there are 
> > > any volatile subobjects.
> > And this is why I don't want to duplicate the logic. :)
> > 
> > I'd rather not make different assumptions for C and C++; instead, I'd 
> > prefer to just use the intersection that's safe in both.  I'm concerned 
> > that we could come up with weird results for mixed C and C++ code, 
> > otherwise.  Also, it's easier to define the C++ rules because we can base 
> > them on the constexpr rules in the standard.
> I agree we probably want the same outcome as D76169 provides, if either the 
> performance is acceptable or we can find a way to avoid the performance cost 
> for the cases we already accept. Perhaps we could get that outcome by 
> ensuring that we try the CGExprConstant fast-path (at least in C 
> compilations, maybe in general) before we consider the complete-but-slow 
> evaluation strategy in ExprConstant?
I like the idea of guarding constant evaluation with a "fast path" that doesn't 
actually compute the APValues in simple cases.  We can just implement the 
simple stuff, and fall back to the full logic for complicated stuff.

My one concern is that we could go to a bunch of effort to emit a variable on 
the fast path, then fall off the fast path later because "return a[0];" tries 
to constant-fold a big array "a".


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 to be constant expressions in initializer lists

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

efriedma wrote:
> rsmith wrote:
> > nickdesaulniers wrote:
> > > efriedma wrote:
> > > > You need to be more careful here; we can call ConstExprEmitter for 
> > > > arbitrary expressions.
> > > "Be more careful" how?
> > Here are some specific cases in which you need to be more careful because 
> > the code as-is would do the wrong thing:
> > 
> >  * when emitting a global's initializer in C++, where the value of the 
> > object denoted by the DeclRefExpr could have changed between its 
> > initialization and the expression we're currently emitting 
> >  * when emitting anything other than a global initializer in C, where the 
> > value of a global could have changed after its emission
> >  * when emitting a reference to a non-global declaration in C (local 
> > variables might change between initialization and use)
> > 
> > We would need to restrict this to the cases where the variable's value 
> > cannot possibly have changed between initialization and use.
> > 
> > In C, that's (mostly) the case for a static storage variable referenced 
> > from the initializer of a static storage variable, for a thread storage 
> > variable referenced from the initializer of a static storage variable, or 
> > for a thread storage variable referenced from the initializer of a thread 
> > storage variable. Even then, this isn't strictly correct in the presence of 
> > DSOs, but I think it should be correct if the two variables are defined in 
> > the same translation unit.
> > 
> > In C++, that's (mostly) the case when the variable is `const` or 
> > `constexpr` and has no mutable subobjects. (There's still the case where 
> > the reference happens outside the lifetime of the object -- for the most 
> > part we can handwave that away by saying it must be UB, but that's not true 
> > in general in the period of construction and period of destruction.)
> > 
> > In both cases, the optimization is (arguably) still wrong if there are any 
> > volatile subobjects.
> And this is why I don't want to duplicate the logic. :)
> 
> I'd rather not make different assumptions for C and C++; instead, I'd prefer 
> to just use the intersection that's safe in both.  I'm concerned that we 
> could come up with weird results for mixed C and C++ code, otherwise.  Also, 
> it's easier to define the C++ rules because we can base them on the constexpr 
> rules in the standard.
I agree we probably want the same outcome as D76169 provides, if either the 
performance is acceptable or we can find a way to avoid the performance cost 
for the cases we already accept. Perhaps we could get that outcome by ensuring 
that we try the CGExprConstant fast-path (at least in C compilations, maybe in 
general) before we consider the complete-but-slow evaluation strategy in 
ExprConstant?


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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

rsmith wrote:
> nickdesaulniers wrote:
> > efriedma wrote:
> > > You need to be more careful here; we can call ConstExprEmitter for 
> > > arbitrary expressions.
> > "Be more careful" how?
> Here are some specific cases in which you need to be more careful because the 
> code as-is would do the wrong thing:
> 
>  * when emitting a global's initializer in C++, where the value of the object 
> denoted by the DeclRefExpr could have changed between its initialization and 
> the expression we're currently emitting 
>  * when emitting anything other than a global initializer in C, where the 
> value of a global could have changed after its emission
>  * when emitting a reference to a non-global declaration in C (local 
> variables might change between initialization and use)
> 
> We would need to restrict this to the cases where the variable's value cannot 
> possibly have changed between initialization and use.
> 
> In C, that's (mostly) the case for a static storage variable referenced from 
> the initializer of a static storage variable, for a thread storage variable 
> referenced from the initializer of a static storage variable, or for a thread 
> storage variable referenced from the initializer of a thread storage 
> variable. Even then, this isn't strictly correct in the presence of DSOs, but 
> I think it should be correct if the two variables are defined in the same 
> translation unit.
> 
> In C++, that's (mostly) the case when the variable is `const` or `constexpr` 
> and has no mutable subobjects. (There's still the case where the reference 
> happens outside the lifetime of the object -- for the most part we can 
> handwave that away by saying it must be UB, but that's not true in general in 
> the period of construction and period of destruction.)
> 
> In both cases, the optimization is (arguably) still wrong if there are any 
> volatile subobjects.
And this is why I don't want to duplicate the logic. :)

I'd rather not make different assumptions for C and C++; instead, I'd prefer to 
just use the intersection that's safe in both.  I'm concerned that we could 
come up with weird results for mixed C and C++ code, otherwise.  Also, it's 
easier to define the C++ rules because we can base them on the constexpr rules 
in the standard.


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 to be constant expressions in initializer lists

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

nickdesaulniers wrote:
> efriedma wrote:
> > You need to be more careful here; we can call ConstExprEmitter for 
> > arbitrary expressions.
> "Be more careful" how?
Here are some specific cases in which you need to be more careful because the 
code as-is would do the wrong thing:

 * when emitting a global's initializer in C++, where the value of the object 
denoted by the DeclRefExpr could have changed between its initialization and 
the expression we're currently emitting 
 * when emitting anything other than a global initializer in C, where the value 
of a global could have changed after its emission
 * when emitting a reference to a non-global declaration in C (local variables 
might change between initialization and use)

We would need to restrict this to the cases where the variable's value cannot 
possibly have changed between initialization and use.

In C, that's (mostly) the case for a static storage variable referenced from 
the initializer of a static storage variable, for a thread storage variable 
referenced from the initializer of a static storage variable, or for a thread 
storage variable referenced from the initializer of a thread storage variable. 
Even then, this isn't strictly correct in the presence of DSOs, but I think it 
should be correct if the two variables are defined in the same translation unit.

In C++, that's (mostly) the case when the variable is `const` or `constexpr` 
and has no mutable subobjects. (There's still the case where the reference 
happens outside the lifetime of the object -- for the most part we can handwave 
that away by saying it must be UB, but that's not true in general in the period 
of construction and period of destruction.)

In both cases, the optimization is (arguably) still wrong if there are any 
volatile subobjects.


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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Isn't that what my patch is doing? (Codegen walking the AST/InitListExpr, 
> generating Constants)?

Yes. The issue is we we don't really want to duplicate the logic.


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

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

- add support for compile time arrays


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/test/CodeGen/const-init.c
  clang/test/Sema/array-init.c
  clang/test/Sema/const-eval.c
  clang/test/Sema/init.c

Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -160,3 +160,36 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t)&bitfieldvar }; // expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {};
+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}}
+};
+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]; // should error
+int PR4517_x3 = PR4517_a[0]; // should error
+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
Index: clang/test/Sema/const-eval.c
===
--- clang/test/Sema/const-eval.c
+++ clang/test/Sema/const-eval.c
@@ -150,5 +150,5 @@
   int arr[];
 };
 int PR35214_x;
-int PR35214_y = ((struct PR35214_X *)&PR35214_x)->arr[1]; // expected-error {{not a compile-time constant}}
+int PR35214_y = ((struct PR35214_X *)&PR35214_x)->arr[1];
 int *PR35214_z = &((struct PR35214_X *)&PR35214_x)->arr[1]; // ok, &PR35214_x + 2
Index: clang/test/Sema/array-init.c
===
--- clang/test/Sema/array-init.c
+++ clang/test/Sema/array-init.c
@@ -268,7 +268,7 @@
   };
 }
 
-char badchararray[1] = { badchararray[0], "asdf" }; // expected-warning {{excess elements in array initializer}} expected-error {{initializer element is not a compile-time constant}}
+char badchararray[1] = { badchararray[0], "asdf" }; // expected-warning {{excess elements in array initializer}}
 
 // Test the GNU extension for initializing an array from an array
 // compound literal. PR9261.
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -181,3 +181,28 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, align 4
 }
+
+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 4
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1007,6 +1007,40 @@
 return Visit(PE->getSubExpr(), T);
   }
 
+  llvm::Constant *VisitDeclRefExpr(DeclRefExpr *DRE, QualType T) {
+if (VarDecl *V = dyn_cast(DRE->getDecl()))
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;
+  }
+
+  llvm::Constant *VisitIntegerLiteral(Int

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#1922239 , @rsmith wrote:

> it's substantially more efficient for CodeGen to walk the AST representation 
> (the `InitListExpr`) and directly generate an IR constant than it is to 
> create an `APValue` representation of the array. (`APValue` is not especially 
> space-efficient, and the extra copying and data shuffling can be quite slow.)


Isn't that what my patch is doing? (Codegen walking the AST/`InitListExpr`, 
generating `Constant`s)?

Uploading what I have; handling arrays is trickier than structs; the index and 
the base both end up having complex subtrees to fetch values from.  As much fun 
as it is to build a compile time evaluator, it sounds like I should stop 
pursing  `CGExprConstant` visitors in favor of `ExprConstant`? (I guess it's 
not clear to me whether @rsmith is in agreement with @eli.friedman on whether 
we want to be more aggressive in compile time evaluation or not).


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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Removing those two LangOpt checks isn't enough for the test cases to run.

Removing those two checks should unblock const-eval for structs in general.  I 
wasn't thinking about whether there something else for global variables 
specifically, though.  It looks like there's a relevant FIXME in 
findCompleteObject() in ExprConstant.cpp.

> APValue is not especially space-efficient

Yes.  We should probably do something similar to what we do for LLVM constants: 
add specialized representations for simple cases, like ConstantDataArray.


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 to be constant expressions in initializer lists

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D76096#1921842 , @nickdesaulniers 
wrote:

> > The performance implications of deleting those lines is the complicated 
> > part.
>
> Where does compile time performance suffer from this? I guess if we have 
> massive array initializers, or large struct definitions, or deeply nested 
> struct definitions, it might take time to recursively evaluate if all members 
> are constant expressions; but isn't that what I want as a developer, to 
> offload the calculations to compile time rather than runtime?  Or is the cost 
> way too significant?


It's exactly what you suspect: very large global arrays initialized from 
constant data. For those, it's substantially more efficient for CodeGen to walk 
the AST representation (the `InitListExpr`) and directly generate an IR 
constant than it is to create an `APValue` representation of the array. 
(`APValue` is not especially space-efficient, and the extra copying and data 
shuffling can be quite slow.) We saw a significant compile time regression on a 
couple of LNT benchmarks without these early bailouts for C.


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D76096#1920685 , @efriedma wrote:

> I think the code that disables constant evaluation for C is just 
> https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc057/clang/lib/AST/ExprConstant.cpp#L13918
>  and 
> https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc057/clang/lib/AST/ExprConstant.cpp#L13744
>  .


Removing those two `LangOpt` checks isn't enough for the test cases to run.


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

efriedma wrote:
> efriedma wrote:
> > nickdesaulniers wrote:
> > > efriedma wrote:
> > > > nickdesaulniers wrote:
> > > > > nickdesaulniers wrote:
> > > > > > Interesting, playing with this more in godbolt, it looks like the 
> > > > > > struct doesn't even have to be const qualified.
> > > > > Or, rather, behaves differently between C and C++ mode;
> > > > > 
> > > > > C -> const required
> > > > > C++ -> const not required
> > > > In C++, global variable initializers don't have to be constant 
> > > > expressions at all.
> > > > 
> > > > Do we really need to check GNUMode here? We try to avoid it except for 
> > > > cases where we would otherwise reject valid code.
> > > > 
> > > > Do we need to worry about arrays here?
> > > > In C++, global variable initializers don't have to be constant 
> > > > expressions at all.
> > > 
> > > It looks like my test cases are supported already in Clang today, in C++ 
> > > mode only and not C.  Maybe there's some alternative code path that I 
> > > should be looking to reuse?
> > > 
> > > > Do we really need to check GNUMode here?
> > > 
> > > Maybe a `-Wpedantic` diag would be more appropriate otherwise? (GCC does 
> > > not warn for these cases with `-Wpedantic`.  If the answer to your 
> > > question is `no`, then that means supporting these regardless of language 
> > > mode.  (I'm ok with that, was just being maybe overly cautious with 
> > > `GNUMode`, but maybe folks with better knowledge of the language 
> > > standards have better thoughts?)
> > > 
> > > > Do we need to worry about arrays here?
> > > 
> > > I don't think arrays are supported: https://godbolt.org/z/RiZPpM
> > Also, do we need to check that we actually have a definition for the 
> > variable?
> The C++ standard is substantially different from C.  C++ global initializers 
> can be evaluated at runtime.  So we don't call this code at all in C++.
> 
> Independent of that, we do have pretty complete support for constant 
> evaluation of structs in C++ to support constexpr, and we should be able to 
> leverage that.
> 
> 
> 
> For arrays, I was thinking of something like this:
> 
> ```
> const int foo[3] = { 0, 1, 2 };
> int bar = foo[0];
> ```
> 
> 
> 
> We generally don't generate pedantic warnings unless the user uses an 
> extension that's disallowed by the C standard.  (The idea is that clang with 
> -pedantic should generate a diagnostic every place the C standard requires a 
> diagnostic.  It's not a catch-all for extensions.)
> 
> We could separately generate some sort of portability warning, but not sure 
> anyone would care to enable it.
> Do we really need to check GNUMode here?

Will remove.

> Do we need to worry about arrays here?

Yep; as long as the base and index are `const` qualified, GCC allows them.  
Will add tests.

> Also, do we need to check that we actually have a definition for the variable?

Yep, will add tests.

> It's not a catch-all for extensions.

Ah, got 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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers planned changes to this revision.
nickdesaulniers added a comment.

> The performance implications of deleting those lines is the complicated part.

Where does compile time performance suffer from this? I guess if we have 
massive array initializers, or large struct definitions, or deeply nested 
struct definitions, it might take time to recursively evaluate if all members 
are constant expressions; but isn't that what I want as a developer, to offload 
the calculations to compile time rather than runtime?  Or is the cost way too 
significant?  Looks like @rsmith added those comments/checks back in 2012 via 
commit dafff947599e ("constexpr irgen: Add irgen support for APValue::Struct, 
APValue::Union,").

Let me see if I comment out the code I've added, then modify those two spots in 
ExprConstant.cpp you pointed out, if that works as well.


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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think the code that disables constant evaluation for C is just 
https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc057/clang/lib/AST/ExprConstant.cpp#L13918
 and 
https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc057/clang/lib/AST/ExprConstant.cpp#L13744
 .  The performance implications of deleting those lines is the complicated 
part.




Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

efriedma wrote:
> nickdesaulniers wrote:
> > efriedma wrote:
> > > nickdesaulniers wrote:
> > > > nickdesaulniers wrote:
> > > > > Interesting, playing with this more in godbolt, it looks like the 
> > > > > struct doesn't even have to be const qualified.
> > > > Or, rather, behaves differently between C and C++ mode;
> > > > 
> > > > C -> const required
> > > > C++ -> const not required
> > > In C++, global variable initializers don't have to be constant 
> > > expressions at all.
> > > 
> > > Do we really need to check GNUMode here? We try to avoid it except for 
> > > cases where we would otherwise reject valid code.
> > > 
> > > Do we need to worry about arrays here?
> > > In C++, global variable initializers don't have to be constant 
> > > expressions at all.
> > 
> > It looks like my test cases are supported already in Clang today, in C++ 
> > mode only and not C.  Maybe there's some alternative code path that I 
> > should be looking to reuse?
> > 
> > > Do we really need to check GNUMode here?
> > 
> > Maybe a `-Wpedantic` diag would be more appropriate otherwise? (GCC does 
> > not warn for these cases with `-Wpedantic`.  If the answer to your question 
> > is `no`, then that means supporting these regardless of language mode.  
> > (I'm ok with that, was just being maybe overly cautious with `GNUMode`, but 
> > maybe folks with better knowledge of the language standards have better 
> > thoughts?)
> > 
> > > Do we need to worry about arrays here?
> > 
> > I don't think arrays are supported: https://godbolt.org/z/RiZPpM
> Also, do we need to check that we actually have a definition for the variable?
The C++ standard is substantially different from C.  C++ global initializers 
can be evaluated at runtime.  So we don't call this code at all in C++.

Independent of that, we do have pretty complete support for constant evaluation 
of structs in C++ to support constexpr, and we should be able to leverage that.



For arrays, I was thinking of something like this:

```
const int foo[3] = { 0, 1, 2 };
int bar = foo[0];
```



We generally don't generate pedantic warnings unless the user uses an extension 
that's disallowed by the C standard.  (The idea is that clang with -pedantic 
should generate a diagnostic every place the C standard requires a diagnostic.  
It's not a catch-all for extensions.)

We could separately generate some sort of portability warning, but not sure 
anyone would care to enable 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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

nickdesaulniers wrote:
> efriedma wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > Interesting, playing with this more in godbolt, it looks like the 
> > > > struct doesn't even have to be const qualified.
> > > Or, rather, behaves differently between C and C++ mode;
> > > 
> > > C -> const required
> > > C++ -> const not required
> > In C++, global variable initializers don't have to be constant expressions 
> > at all.
> > 
> > Do we really need to check GNUMode here? We try to avoid it except for 
> > cases where we would otherwise reject valid code.
> > 
> > Do we need to worry about arrays here?
> > In C++, global variable initializers don't have to be constant expressions 
> > at all.
> 
> It looks like my test cases are supported already in Clang today, in C++ mode 
> only and not C.  Maybe there's some alternative code path that I should be 
> looking to reuse?
> 
> > Do we really need to check GNUMode here?
> 
> Maybe a `-Wpedantic` diag would be more appropriate otherwise? (GCC does not 
> warn for these cases with `-Wpedantic`.  If the answer to your question is 
> `no`, then that means supporting these regardless of language mode.  (I'm ok 
> with that, was just being maybe overly cautious with `GNUMode`, but maybe 
> folks with better knowledge of the language standards have better thoughts?)
> 
> > Do we need to worry about arrays here?
> 
> I don't think arrays are supported: https://godbolt.org/z/RiZPpM
Also, do we need to check that we actually have a definition for the variable?


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked an inline comment as done and an inline comment as not 
done.
nickdesaulniers added a comment.

In D76096#1920477 , @efriedma wrote:

> But that's probably a larger project than you really want to mess with.


Maybe with some coaching; but if this is already supported today in C++, maybe 
it's just a small incision to support this in C?




Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

efriedma wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > Interesting, playing with this more in godbolt, it looks like the struct 
> > > doesn't even have to be const qualified.
> > Or, rather, behaves differently between C and C++ mode;
> > 
> > C -> const required
> > C++ -> const not required
> In C++, global variable initializers don't have to be constant expressions at 
> all.
> 
> Do we really need to check GNUMode here? We try to avoid it except for cases 
> where we would otherwise reject valid code.
> 
> Do we need to worry about arrays here?
> In C++, global variable initializers don't have to be constant expressions at 
> all.

It looks like my test cases are supported already in Clang today, in C++ mode 
only and not C.  Maybe there's some alternative code path that I should be 
looking to reuse?

> Do we really need to check GNUMode here?

Maybe a `-Wpedantic` diag would be more appropriate otherwise? (GCC does not 
warn for these cases with `-Wpedantic`.  If the answer to your question is 
`no`, then that means supporting these regardless of language mode.  (I'm ok 
with that, was just being maybe overly cautious with `GNUMode`, but maybe folks 
with better knowledge of the language standards have better thoughts?)

> Do we need to worry about arrays here?

I don't think arrays are supported: https://godbolt.org/z/RiZPpM



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

efriedma wrote:
> You need to be more careful here; we can call ConstExprEmitter for arbitrary 
> expressions.
"Be more careful" how?


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 to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I would like to kill off all the code you're modifying, and let ExprConstant be 
the final arbiter of whether a constant is in fact constant.  But that's 
probably a larger project than you really want to mess with.  The big missing 
piece of that is that we currently don't allow ExprConstant to evaluate 
structs/arrays in C, for the sake of compile-time performance. (Not sure what 
the performance impact for large arrays looks like at the moment.)




Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > Interesting, playing with this more in godbolt, it looks like the struct 
> > doesn't even have to be const qualified.
> Or, rather, behaves differently between C and C++ mode;
> 
> C -> const required
> C++ -> const not required
In C++, global variable initializers don't have to be constant expressions at 
all.

Do we really need to check GNUMode here? We try to avoid it except for cases 
where we would otherwise reject valid code.

Do we need to worry about arrays here?



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

You need to be more careful here; we can call ConstExprEmitter for arbitrary 
expressions.


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.



Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

Interesting, playing with this more in godbolt, it looks like the struct 
doesn't even have to be const qualified.


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

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

- add 2 missing CHECKs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/test/CodeGen/const-init.c
  clang/test/Sema/init.c


Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -160,3 +160,22 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t)&bitfieldvar }; // 
expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {};
+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
+};
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -181,3 +181,20 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+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
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1007,6 +1007,13 @@
 return Visit(PE->getSubExpr(), T);
   }
 
+  llvm::Constant *VisitDeclRefExpr(DeclRefExpr *DRE, QualType T) {
+if (VarDecl *V = dyn_cast(DRE->getDecl()))
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;
+  }
+
   llvm::Constant *
   VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *PE,
 QualType T) {
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -3158,6 +3158,13 @@
 
   switch (getStmtClass()) {
   default: break;
+  case DeclRefExprClass:
+if (!Ctx.getLangOpts().CPlusPlus && Ctx.getLangOpts().GNUMode) {
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;
+}
+break;
   case StringLiteralClass:
   case ObjCEncodeExprClass:
 return true;


Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -160,3 +160,22 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t)&bitfieldvar }; // expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {};
+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
+};
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -181,3 +181,20 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, align 4
 }
+
+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 

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked an inline comment as not done.
nickdesaulniers added inline comments.



Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

nickdesaulniers wrote:
> Interesting, playing with this more in godbolt, it looks like the struct 
> doesn't even have to be const qualified.
Or, rather, behaves differently between C and C++ mode;

C -> const required
C++ -> const not required


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 to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: eli.friedman, aaron.ballman, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nickdesaulniers updated this revision to Diff 250046.
nickdesaulniers added a comment.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added subscribers: void, jyknight.
nickdesaulniers marked an inline comment as not done.

- add 2 missing CHECKs




Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

Interesting, playing with this more in godbolt, it looks like the struct 
doesn't even have to be const qualified.



Comment at: clang/lib/AST/Expr.cpp:3164
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;

nickdesaulniers wrote:
> Interesting, playing with this more in godbolt, it looks like the struct 
> doesn't even have to be const qualified.
Or, rather, behaves differently between C and C++ mode;

C -> const required
C++ -> const not required


This seems to be an undocumented GNU C extension.

For code like:
struct foo { ... };
struct bar { struct foo foo; };
struct foo my_foo = { ... };
struct bar my_bar = { .foo = my_foo };

during CodeGen of LLVM IR, copy the initializer of `my_foo` into the
initializer of `my_bar` recursively.

Eli Friedman points out the relevant part of the C standard seems to
have some flexibility in what is considered a constant expression:

6.6 paragraph 10:
An implementation may accept other forms of constant expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76096

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/test/CodeGen/const-init.c
  clang/test/Sema/init.c


Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -160,3 +160,22 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWithBitfield bitfieldvar = { (uintptr_t)&bitfieldvar }; // 
expected-error {{initializer element is not a compile-time constant}}
+
+// PR45157
+struct PR4517_foo {};
+struct PR4517_bar {
+  struct PR4517_foo foo;
+};
+const struct PR4517_foo my_foo = {};
+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
+};
Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -181,3 +181,20 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+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
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1007,6 +1007,13 @@
 return Visit(PE->getSubExpr(), T);
   }
 
+  llvm::Constant *VisitDeclRefExpr(DeclRefExpr *DRE, QualType T) {
+if (VarDecl *V = dyn_cast(DRE->getDecl()))
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;
+  }
+
   llvm::Constant *
   VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *PE,
 QualType T) {
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -3158,6 +3158,13 @@
 
   switch (getStmtClass()) {
   default: break;
+  case DeclRefExprClass:
+if (!Ctx.getLangOpts().CPlusPlus && Ctx.getLangOpts().GNUMode) {
+  const QualType &QT = cast(this)->getDecl()->getType();
+  if (QT->isStructureType() && QT.isConstQualified())
+return true;
+}
+break;
   case StringLiteralClass:
   c