os_log: make buffer size an integer constant expression.

[t.p.northover]

Summary

The size of an os_log buffer is known at any stage of compilation, so making it a constant expression means that the common idiom of declaring a buffer for it won't result in a VLA. That allows the compiler to skip saving and restoring the stack pointer around such buffers.

There can be hundreds or even thousands of such calls, even in firmware projects, so the code size savings add up.

[t.p.northover]

Ping.

[efriedma]

LGTM

[t.p.northover]

Thanks Eli. I committed it as r345828, and then had to fixup some linker dependencies on other platforms, which took me a couple of tries (r345833 and r345835).

[t.p.northover]

Turns out I neglected the layering between libclangAST and libclangAnalysis so I've reverted for now. For this to work I think we need to move the OSLog helpers into libclangAST. I'll put together a new patch and upload it.

[t.p.northover]

Same as previous patch except the OSLog helpers are moved to libclangAST to respect dependencies.

[efriedma]

LGTM

[hiraditya]

closed in r345828, 314fbfa1c4c6665c54a220eefb10a6f23010a352

clang/lib/AST/ExprConstant.cpp

 //
 // If we are checking for a potential constant expression, failure to constant
 // fold a potential constant sub-expression will be indicated by a 'false'
 // return value (the expression could not be folded) and no diagnostic (the
 // expression is not necessarily non-constant).
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Analysis/Analyses/OSLog.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
case Builtin::BI__builtin_object_size: {
     case EvalInfo::EM_PotentialConstantExpressionUnevaluated:
       // Reduce it to a constant now.
       return Success((Type & 2) ? 0 : -1, E);
     }
 
     llvm_unreachable("unexpected EvalMode");
   }
 
+  case Builtin::BI__builtin_os_log_format_buffer_size: {
+    analyze_os_log::OSLogBufferLayout Layout;
+    analyze_os_log::computeOSLogBufferLayout(Info.Ctx, E, Layout);
+    return Success(Layout.size().getQuantity(), E);
+  }
+
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
   case Builtin::BI__builtin_bswap64: {
     APSInt Val;
     if (!EvaluateInteger(E->getArg(0), Val, Info))
       return false;
 
     return Success(Val.byteSwap(), E);

clang/lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__builtin_thread_pointer: {
     if (!getContext().getTargetInfo().isTLSSupported())
       CGM.ErrorUnsupported(E, "__builtin_thread_pointer");
     // Fall through - it's already mapped to the intrinsic by GCCBuiltin.
     break;
   }
   case Builtin::BI__builtin_os_log_format:
     return emitBuiltinOSLogFormat(*E);
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-    analyze_os_log::OSLogBufferLayout Layout;
-    analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
-    return RValue::get(ConstantInt::get(ConvertType(E->getType()),
-                                        Layout.size().getQuantity()));
-  }
-
   case Builtin::BI__xray_customevent: {
     if (!ShouldXRayInstrumentFunction())
       return RValue::getIgnored();
 
     if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
             XRayInstrKind::Custom))
       return RValue::getIgnored();
 

clang/test/CodeGen/builtins.c

 void test_builtin_os_log_merge_helper1(void *buf, unsigned u, long long ll) {
   __builtin_os_log_format(buf, "%u %lld", u, ll);
 }
 
 // Check that this function doesn't write past the end of array 'buf'.
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* %[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8* %[[ARGSIZE]], align 1
 // CHECK-NEXT: ret void
 
 // CHECK-LABEL: define void @test_builtin_os_log_long_double
 // CHECK: (i8* %[[BUF:.*]], x86_fp80 %[[LD:.*]])
 void test_builtin_os_log_long_double(void *buf, long double ld) {
   // CHECK: %[[BUF_ADDR:.*]] = alloca i8*, align 8