<div><br></div><div style="background-color:rgba(0,0,0,0)!important;border-color:rgb(255,255,255)!important;color:rgb(255,255,255)!important"><br><div class="gmail_quote" style="background-color:rgba(0,0,0,0)!important;border-color:rgb(255,255,255)!important;color:rgb(255,255,255)!important"><div dir="ltr" class="gmail_attr">On Tue, Jul 18, 2023 at 8:41 PM serge via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)">serge-sans-paille created this revision.<br>
serge-sans-paille added reviewers: nickdesaulniers, jfb, aaron.ballman.<br>
Herald added a subscriber: kristof.beyls.<br>
Herald added a project: All.<br>
serge-sans-paille requested review of this revision.<br>
Herald added a project: clang.<br>
Herald added a subscriber: cfe-commits.<br>
<br>
Empty class still use one byte, but there is no way to read that byte<br>
programmatically in a legitimate way. Yet trivial auto var init always<br>
generate a store for it and there is no programmatic way to prevent the<br>
generation of that extra store.</blockquote><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">This is the same as padding, and is initialized on purpose. If it’s truly unused then the optimizer will eliminate it… unless it can’t prove whether the store is dead. </div><div dir="auto"><br></div><div dir="auto">Does this show up in any meaningless performance case? If so, can we optimize it away?</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-color:rgb(255,255,255) rgb(255,255,255) rgb(255,255,255) rgb(74,77,80)!important;background-color:rgba(0,0,0,0)!important;color:rgb(255,255,255)!important" dir="auto"><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
This patch harmonizes Clang behavior with GCC and does not generate</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
initialization code for empty classes under -ftrivial-auto-var-init</font><br>
<br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
Repository:</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
  rG LLVM Github Monorepo</font><br>
<br>
<a href="https://reviews.llvm.org/D155580" rel="noreferrer" target="_blank">https://reviews.llvm.org/D155580</a><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
Files:</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
  clang/lib/CodeGen/CGDecl.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
  clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init-empty-class.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
  clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init.cpp</font><br>
<br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
Index: clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
==============================</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">==============================</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">=======</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
--- clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+++ clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
@@ -43,9 +43,6 @@</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // PATTERN-NOT: undef</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // ZERO-NOT: undef</font><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// PATTERN-O0: @__const.test_empty_uninit.uni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">nit = private unnamed_addr constant %struct.empty { i8 [[I8]] }, align 1</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// PATTERN-O1-NOT: @__const.test_empty_uninit.uni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">nit</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-struct empty {};</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // PATTERN-O0: @__const.test_small_uninit.uni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">nit = private unnamed_addr constant %struct.small { i8 [[I8]] }, align 1</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // PATTERN-O0: @__const.test_small_custom.cus</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">tom = private unnamed_addr constant %struct.small { i8 42 }, align 1</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // ZERO-O0: @__const.test_small_custom.cus</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">tom = private unnamed_addr constant %struct.small { i8 42 }, align 1</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
@@ -586,24 +583,6 @@</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // CHECK-NOT:   !annotation</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)</font><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-TEST_UNINIT(empty, empty);</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK-LABEL: @test_empty_uninit()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK:       %uninit = alloca %struct.empty, align</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// PATTERN-LABEL: @test_empty_uninit()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_empty_uninit.uni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">nit{{.+}}), !annotation [[AUTO_INIT]]</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// PATTERN-O1: store i8 [[I8]], {{.*}} align 1, !annotation [[AUTO_INIT]]</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// ZERO-LABEL: @test_empty_uninit()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// ZERO-O0: call void @llvm.memset{{.*}}, i8 0,{{.+}}), !annotation [[AUTO_INIT]]</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// ZERO-O1: store i8 0, {{.*}} align 1, !annotation [[AUTO_INIT]]</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-TEST_BRACES(empty, empty);</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK-LABEL: @test_empty_braces()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK:       %braces = alloca %struct.empty, align</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK-NEXT:  call void @llvm.memcpy</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK-NOT:   !annotation</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 TEST_UNINIT(small, small);</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // CHECK-LABEL: @test_small_uninit()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 // CHECK:       %uninit = alloca %struct.small, align</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
Index: clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init-empty-class.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
==============================</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">==============================</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">=======</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
--- /dev/null</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+++ clang/test/CodeGenCXX/auto-var</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">-init-empty-class.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
@@ -0,0 +1,18 @@</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=patter</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">n %s -emit-llvm -o - | FileCheck %s</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// C++ empty classes still take some memory, but there is no need to initialize</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// it has it cannot be accessed. This matches GCC behavior.</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+struct S {</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+};</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+extern "C" void use(struct S*);</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// CHECK-LABEL: @empty_class(</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// CHECK:       %s = alloca</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+// CHECK-NEXT:  call void @use(ptr noundef %s)</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+extern "C" void empty_class(void) {</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  S s;</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  return use(&s);</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+}</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
Index: clang/lib/CodeGen/CGDecl.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
==============================</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">==============================</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">=======</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
--- clang/lib/CodeGen/CGDecl.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+++ clang/lib/CodeGen/CGDecl.cpp</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
@@ -1855,6 +1855,13 @@</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
   }</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 }</font><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+static bool isEmptyClass(VarDecl const &D) {</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  const Type *Ty = D.getType().getTypePtr();</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  if (const auto *CxxRecordTy = Ty->getAsCXXRecordDecl())</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+    return CxxRecordTy->isEmpty();</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  return false;</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+}</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
 void CodeGenFunction::EmitAutoVarIn</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">it(const AutoVarEmission &emission) {</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
   assert(emission.Variable && "emission was not valid!");</font><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
@@ -1903,12 +1910,18 @@</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
       locIsByrefHeader ? emission.getObjectAddress(*thi</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">s) : emission.Addr;</font><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
   // Note: constexpr already initializes everything correctly.</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  // Note: empty classes still use a byte, but it's ok not to initialize it as</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+  // it cannot be legitimately accessed.</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
   LangOptions::TrivialAutoVarIn</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">itKind trivialAutoVarInit =</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
       (D.isConstexpr()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
            ? LangOptions::TrivialAutoVarIni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">tKind::Uninitialized</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
            : (D.getAttr<UninitializedAttr>(</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">)</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
                   ? LangOptions::TrivialAutoVarIni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">tKind::Uninitialized</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
-                  : getContext().getLangOpts().get</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">TrivialAutoVarInit()));</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+                  : (isEmptyClass(D)</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+                         ? LangOptions::TrivialAutoVarIni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">tKind::Uninitialized</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+                         : getContext()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+                               .getLangOpts()</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
+                               .getTrivialAutoVarInit())));</font><br>
<br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
   auto initializeWhatIsTechnicallyUni</font><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">nitialized = [&](Address Loc) {</font><br><font style="border-left-color:rgb(204,204,204);color:rgb(0,0,0)">
     if (trivialAutoVarInit ==</font><br>
<br>
<br>
</blockquote></div></div>