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

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D54355#1328786 <https://reviews.llvm.org/D54355#1328786>, @jyknight wrote:

> In D54355#1328557 <https://reviews.llvm.org/D54355#1328557>, @void wrote:
>
> > The issue is that "`n`" is expecting an immediate value, but the result of the ternary operator isn't calculated by the front-end, because it doesn't "know" that the evaluation of `__builtin_constant_p` shouldn't be delayed (it being compiled at `-O0`). I suspect that this issue will also happen with the "`i`" constraint.
>
>
> Indeed. We _do_ actually already know that in clang too, however -- clang already knows that "n" requires an immediate, and does constant expression evaluation for it, e.g. to expand constexpr functions. Grep for "requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p correctly, though.
>
> (Note, as a workaround, you could use `|` instead of `||`, or put the expression as the value of an enumeration constant).


I looked at `requiresImmediateConstant`, but when I set it for `n` and `i` things broke all over the place. I sent out a patch for review. It's limited to `-O0`, since we can compile with optimizations without complaint. And there may be instances where it's necessary:

  inline void foo(int x) {
    asm("" : : "n" (__builtin_constant_p(n) ? 1 : -1));
  }
  
  void bar() {
    foo(37);
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355