Improved __builtin_constant_p

[• janusz]

Summary

Improved __builtin_constant_p() implementation to match GCC closer.
If the compiler cannot confirm that the argument is constant, the function
is lowered to a (new) llvm.is.constant intrinsic and the constantness is
evaluated again after optimisations such as inlining.

Depends on D4276

[rsmith]

This makes Clang's constant expression evaluator and its IR generation disagree about the value of __builtin_constant_p. That seems dangerous to me, so I'd like to understand the justification for this a bit better. Random example of why this is dangerous:

int f(int n) {
  if (!__builtin_constant_p(n))
    return 0;
}
int k = f(0);

Here, we'll suppress the -Wreturn-type warning because the frontend believes that it's impossible to reach the } of f. But when we generate IR, we'll fall off the function and experience nasal demons.

What's the motivation behind this change?

[• janusz]

In D4492#5, @rsmith wrote:

This makes Clang's constant expression evaluator and its IR generation disagree about the value of __builtin_constant_p.

How would you address this issue? My concern is that there are cases, where we need to fold to a constant early (i.e. global variable initialisation) and other cases where we can emit IR code and make the decision about 'constantness' at a later stage.

What's the motivation behind this change?

Matching GCC behaviour as closely as we can.

lib/CodeGen/CGBuiltin.cpp

assert(X->getType() == Y->getType() &&
          "arguments have the same integer width?)");
 
   llvm::Value *Callee = CGF.CGM.getIntrinsic(IntrinsicID, X->getType());
   llvm::Value *Tmp = CGF.Builder.CreateCall2(Callee, X, Y);
   Carry = CGF.Builder.CreateExtractValue(Tmp, 1);
   return CGF.Builder.CreateExtractValue(Tmp, 0);
 }
 
+/// \brief Emit a call to llvm.is.constant.* intrinsic.
+/// If the argument to __builtin_constant_p() is known to be constant,
+/// or is not integral, the function returns the constant that
+/// __builtin_constant_p() folded to.
+/// Otherwise the functions emits llvm.is.constant intrinsic.
+///
+/// \arg CGF The current codegen function.
+/// \arg Result The Result, the constant __builtin_constant_p() folded to.
+/// \arg E The __builtin_constant_p() expression.
+/// \returns The result returned by the intrinsic.
+static RValue EmitIsConstantIntrinsic(CodeGenFunction &CGF,
+                               Expr::EvalResult &Result,
+                               const CallExpr *E) {
+    APSInt val = Result.Val.getInt();
+    Value *RetVal = NULL;
+    // FIXME: Add support for non-integral arguments
+    if ((val == 0) && (E->getArg(0)->getType()->isIntegralOrEnumerationType())) {
+      Value *ArgValue = CGF.EmitScalarExpr(E->getArg(0));
+      llvm::Type *ArgType = ArgValue->getType();
+      Value *F = CGF.CGM.getIntrinsic(Intrinsic::is_constant, ArgType);
+      llvm::Type *ResultType = CGF.ConvertType(E->getType());
+      RetVal = CGF.Builder.CreateCall(F, ArgValue);
+      if (RetVal->getType() != ResultType)
+        RetVal = CGF.Builder.CreateIntCast(RetVal, ResultType, /*isSigned*/false,
+                                           "cast");
+    } else {
+      RetVal = llvm::ConstantInt::get(CGF.getLLVMContext(), val);
+    }
+
+    return RValue::get(RetVal);
+  }
+
+
 RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
                                         unsigned BuiltinID, const CallExpr *E) {
   // See if we can constant fold this builtin.  If so, don't emit it at all.
   Expr::EvalResult Result;
   if (E->EvaluateAsRValue(Result, CGM.getContext()) &&
       !Result.hasSideEffects()) {
-    if (Result.Val.isInt())
+    if (Result.Val.isInt()) {
+      if (BuiltinID == Builtin::BI__builtin_constant_p)
+        return EmitIsConstantIntrinsic(*this, Result, E);
+      else
         return RValue::get(llvm::ConstantInt::get(getLLVMContext(),
                                                 Result.Val.getInt()));
+    }
     if (Result.Val.isFloat())
       return RValue::get(llvm::ConstantFP::get(getLLVMContext(),
                                                Result.Val.getFloat()));
   }
 
   switch (BuiltinID) {
   default: break;  // Handle intrinsics and libm functions below.
   case Builtin::BI__builtin___CFStringMakeConstantString:

test/CodeGen/builtin_constant_p.c

+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+// Test verifies that __builtin_constant_p() is lowered to
+// llvm.is.constant intrinsic if the argument is not known
+// to be constant.
+// CHECK: @test1
+// CHECK: @llvm.is.constant
+// CHECK: {{ *ret }}
+int test1(int x) {
+  return __builtin_constant_p(x);
+}
+
+// If the argument is known to be constant, __builtin_constant_p()
+// is lowered to a constant (1)
+// CHECK: @test2
+// CHECK-NOT: @llvm.is.constant
+// CHECK: {{ *ret i[0-9]+ 1}}
+int test2() {
+  return __builtin_constant_p(12);
+}

test/Sema/builtin_constant_p.cpp

+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic
+// Check if __builtin_constant_p can be used in C++ context.
+
+// expected-no-diagnostics
+
+// Change this to '1' to convert it to an executable program.
+#define PRINT_VALUES 0
+
+#if PRINT_VALUES
+#include <iostream>
+#endif
+
+class test_class {
+public:
+  test_class(int v) : v1(__builtin_constant_p(v)), v2(__builtin_constant_p(1)), v3(v) {
+    v4 = 2 + __builtin_constant_p(v + 1);
+    v5 = 3 + v4;
+    v6 = v1 + __builtin_constant_p(v3);
+  }
+
+  static int s1, s2, s3;
+  const int v1, v2, v3;
+  int v4, v5, v6;
+
+#if PRINT_VALUES
+  bool is_constructed_from_constant() const {
+    if (v1 == 1 && v2 == 1 && v4 == 3 && v5 == 6 && v6 == 2)
+      return true;
+    return false;
+  }
+#endif
+};
+
+int x;
+const int y = 12;
+
+int test_class::s1 = __builtin_constant_p(0);
+int test_class::s2 = __builtin_constant_p(x);
+int test_class::s3 = __builtin_constant_p(y);
+
+const test_class A1(10);
+const test_class A2(y);
+
+int fun(int x) {
+  return x;
+}
+
+int test(int val) {
+  test_class B1(0);
+  test_class B2(x);
+  test_class B3(y);
+  const test_class B4(x);
+  const test_class B5(y);
+  test_class B6(val);
+  test_class B7(__builtin_constant_p(B1));
+  test_class B8(__builtin_constant_p(fun(0)));
+
+#if PRINT_VALUES
+#define xstr(s) str(s)
+#define str(s) #s
+#define P(X) std::cout << xstr(X) << ": " << X.is_constructed_from_constant() << "\n"
+  std::cout << "s1: " << test_class::s1 << "\n";
+  std::cout << "s2: " << test_class::s2 << "\n";
+  std::cout << "s3: " << test_class::s3 << "\n";
+  P(A1);
+  P(A2);
+  P(B1);
+  P(B2);
+  P(B3);
+  P(B4);
+  P(B5);
+  P(B6);
+  P(B7);
+  P(B8);
+#endif
+  return val;
+}
+
+#if PRINT_VALUES
+int main() {
+  return test(0);
+}
+#endif