[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: nickdesaulniers, jfb, aaron.ballman.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Empty class still use one byte, but there is no way to read that byte
programmatically in a legitimate way. Yet trivial auto var init always
generate a store for it and there is no programmatic way to prevent the
generation of that extra store.

This patch harmonizes Clang behavior with GCC and does not generate
initialization code for empty classes under -ftrivial-auto-var-init


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155580

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/auto-var-init-empty-class.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp


Index: clang/test/CodeGenCXX/auto-var-init.cpp
===================================================================
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -43,9 +43,6 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 [[I8]] }, align 1
-// PATTERN-O1-NOT: @__const.test_empty_uninit.uninit
-struct empty {};
 // PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 [[I8]] }, align 1
 // PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 // ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
@@ -586,24 +583,6 @@
 // CHECK-NOT:   !annotation
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
-TEST_UNINIT(empty, empty);
-// CHECK-LABEL: @test_empty_uninit()
-// CHECK:       %uninit = alloca %struct.empty, align
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
-// PATTERN-LABEL: @test_empty_uninit()
-// PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_empty_uninit.uninit{{.+}}), !annotation [[AUTO_INIT]]
-// PATTERN-O1: store i8 [[I8]], {{.*}} align 1, !annotation [[AUTO_INIT]]
-// ZERO-LABEL: @test_empty_uninit()
-// ZERO-O0: call void @llvm.memset{{.*}}, i8 0,{{.+}}), !annotation [[AUTO_INIT]]
-// ZERO-O1: store i8 0, {{.*}} align 1, !annotation [[AUTO_INIT]]
-
-TEST_BRACES(empty, empty);
-// CHECK-LABEL: @test_empty_braces()
-// CHECK:       %braces = alloca %struct.empty, align
-// CHECK-NEXT:  call void @llvm.memcpy
-// CHECK-NOT:   !annotation
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
-
 TEST_UNINIT(small, small);
 // CHECK-LABEL: @test_small_uninit()
 // CHECK:       %uninit = alloca %struct.small, align
Index: clang/test/CodeGenCXX/auto-var-init-empty-class.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/auto-var-init-empty-class.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s
+
+// C++ empty classes still take some memory, but there is no need to initialize
+// it has it cannot be accessed. This matches GCC behavior.
+
+struct S {
+};
+
+extern "C" void use(struct S*);
+
+// CHECK-LABEL: @empty_class(
+// CHECK:       %s = alloca
+// CHECK-NEXT:  call void @use(ptr noundef %s)
+extern "C" void empty_class(void) {
+  S s;
+  return use(&s);
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===================================================================
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1855,6 +1855,13 @@
   }
 }
 
+static bool isEmptyClass(VarDecl const &D) {
+  const Type *Ty = D.getType().getTypePtr();
+  if (const auto *CxxRecordTy = Ty->getAsCXXRecordDecl())
+    return CxxRecordTy->isEmpty();
+  return false;
+}
+
 void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) {
   assert(emission.Variable && "emission was not valid!");
 
@@ -1903,12 +1910,18 @@
       locIsByrefHeader ? emission.getObjectAddress(*this) : emission.Addr;
 
   // Note: constexpr already initializes everything correctly.
+  // Note: empty classes still use a byte, but it's ok not to initialize it as
+  // it cannot be legitimately accessed.
   LangOptions::TrivialAutoVarInitKind trivialAutoVarInit =
       (D.isConstexpr()
            ? LangOptions::TrivialAutoVarInitKind::Uninitialized
            : (D.getAttr<UninitializedAttr>()
                   ? LangOptions::TrivialAutoVarInitKind::Uninitialized
-                  : getContext().getLangOpts().getTrivialAutoVarInit()));
+                  : (isEmptyClass(D)
+                         ? LangOptions::TrivialAutoVarInitKind::Uninitialized
+                         : getContext()
+                               .getLangOpts()
+                               .getTrivialAutoVarInit())));
 
   auto initializeWhatIsTechnicallyUninitialized = [&](Address Loc) {
     if (trivialAutoVarInit ==


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155580.541457.patch
Type: text/x-patch
Size: 4230 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230718/393504d4/attachment-0001.bin>