[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D53514

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c


Index: clang/test/CodeGen/builtins.c
===================================================================
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // 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
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3594,13 +3594,6 @@
   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();
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -33,6 +33,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Analysis/Analyses/OSLog.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
@@ -8112,6 +8113,12 @@
     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: {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53514.170453.patch
Type: text/x-patch
Size: 3356 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181022/3beec6d4/attachment.bin>