[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 6 inline comments as done.
EricWF added inline comments.


================
Comment at: test/CodeGenCXX/builtin-is-constant-evaluated.cpp:41-42
+
+// CHECK-STATIC-DAG: @p = global i32 26,
+CONSTINIT int p = f(); // f().m == 13; initialized to 26
+
----------------
rsmith wrote:
> Do we get the same result without the attribute?
Yes it does. But Ill ad a second test to verify. 


================
Comment at: test/CodeGenCXX/builtin-is-constant-evaluated.cpp:73
+  // CHECK-ARR: %x1 = alloca [101 x i8],
+  char x1[std::is_constant_evaluated() || __builtin_is_constant_evaluated() ? 101 : 1];
+
----------------
rsmith wrote:
> Why call both the std:: function and the builtin here? If we want to test that both work, shouldn't we use `&&` instead of `||`?
I think I was trying to ensure `std::is_constant_evaluated` worked the same as calling the builtin directly.
Using `||` here was a mistake. Fixed.


================
Comment at: test/CodeGenCXX/builtin-is-constant-evaluated.cpp:91
+bool test_constant_initialized_local(int k) {
+  // CHECK-FOLD: store i8 0, i8* %n,
+  // CHECK-FOLD: store volatile i8* %n, i8** %p,
----------------
rsmith wrote:
> This is incorrect: `n` should be initialized to `true` because it occurs in the initializer of a variable that is usable in constant expressions.
Ack.


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

https://reviews.llvm.org/D55500