[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

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

rsmith added inline comments.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:1930-1931
+    if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+      // At -O0, we don't perform inlining, so we don't need to delay the
+      // processing.
+      return RValue::get(ConstantInt::get(Int32Ty, 0));
----------------
Are there cases where a call to an `always_inline` function could change the outcome?


================
Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+    if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+      Arg = ConstantExpr::Create(Context, Arg);
+
----------------
void wrote:
> rsmith wrote:
> > We should create this node as part of checking that the argument is an ICE (in `SemaBuiltinConstantArg`).
> It turns out that `SemaBuiltinConstantArg` isn't called for `__builtin_constant_p()`. Its argument type is `.`, which...I'm not sure what that means. It looks like a vararg, but that doesn't make sense for the builtin.
> 
> Should I change its signature in `Builtins.def`?
It's also marked `t`, which means "signature is meaningless, uses custom type checking".

The argument to `__builtin_constant_p` isn't required to be an ICE, though, so we shouldn't be calling `SemaBuiltinConstantArg` for it / creating a `ConstantExpr` wrapping it, as far as I can tell.


Repository:
  rC Clang

https://reviews.llvm.org/D54355