Variable auto-init: split out small arrays

[jfb]

Summary

Following up with r355181, initialize small arrays as well.

[jfb]

• typo

[jfb]

I'll do a few size diffs to double-check that this also pays off. @glider can you also check that it doesn't regress what you've been looking at?

test/CodeGenCXX/auto-var-init.cpp

1133 ↗

(On Diff #189084)

This was a typo, inadvertently capturing %0.

[glider]

test/CodeGenCXX/auto-var-init.cpp

1025 ↗

(On Diff #189085)

This check fails for me locally.

[jfb]

• Fix test labels.

[jfb]

test/CodeGenCXX/auto-var-init.cpp

1025 ↗

(On Diff #189085)

Apologies, I played around with the labels and forgot to fix them before sending the patch.

[glider]

The change itself looks good.
It doesn't seem to regress kernel performance on ARM64. I haven't got to testing on x86 yet, but don't anticipate any problems either.

lib/CodeGen/CGDecl.cpp

1206 ↗

(On Diff #189159)

Is the second expression being moved to line 1206 a result of clang-format? Otherwise it'll migrate back at some point.

[jfb]

lib/CodeGen/CGDecl.cpp

1206 ↗

(On Diff #189159)

Yes, the slightly longer name pushes it past 80 columns, and I just auto-format stuff before uploading.

I can do this change separately, I just noticed that the name I originally used was now misleading because vectors aren't scalars :)

[glider]

lib/CodeGen/CGDecl.cpp

1206 ↗

(On Diff #189159)

Up to you, this doesn't matter IMO :)

[jfb]

Comparing clang stage2 in release mode, with an without this change, we see a 408 byte size difference, which is ~nothing. Here's details, nothing surprising:

$ /s/bloaty/bloaty -d sections /s/llvm1/llvm/stage2/bin/clang-9  -- /s/llvm2/llvm/stage2/bin/clang-9
     VM SIZE                                 FILE SIZE
 --------------                           --------------
   +44% +1.84Ki [__TEXT]                  +1.84Ki   +45%
   +65%    +408 [__LINKEDIT]                    0  [ = ]
  -0.0%      -8 Table of Non-instructions      -8  -0.0%
  -0.0%     -48 Symbol Table                  -48  -0.0%
  -0.0%    -120 Export Info                  -120  -0.0%
  -0.0%    -232 String Table                 -232  -0.0%
  -0.0%    -832 __TEXT,__text                -832  -0.0%
  -0.0% -1.03Ki __TEXT,__cstring          -1.03Ki  -0.0%
  [ = ]       0 TOTAL                        -408  -0.0%

$ /s/bloaty/bloaty -d symbols /s/llvm1/llvm/stage2/bin/clang-9  -- /s/llvm2/llvm/stage2/bin/clang-9
     VM SIZE                                                                  FILE SIZE
 --------------                                                            --------------
   +45% +1.84Ki [__TEXT]                                                   +1.84Ki   +45%
  +0.0%    +280 [__LINKEDIT]                                                  -128  -0.0%
  +0.4%     +96 clang::ento::check::ASTDecl<>::_checkDecl<>()                  +96  +0.4%
  +2.9%     +48 emitStoresForConstant()                                        +48  +2.9%
  -0.9%     -16 clang::Sema::DeclareGlobalAllocationFunction()                 -16  -0.9%
  -0.0%     -16 clang::ento::check::PostStmt<>::_checkStmt<>()                 -16  -0.0%
  -8.3%     -78 clang::AnalyzerOptions::getCheckerStringOption()               -78  -8.3%
 -12.5%     -80 clang::ento::registerPaddingChecker()                          -80 -12.5%
  -6.3%     -96 clang::ASTContext::GetBuiltinType()                            -96  -6.3%
 -37.8%    -205 clang::AnalyzerOptions::getCheckerIntegerOption()             -205 -37.8%
 -49.5%    -269 clang::AnalyzerOptions::getCheckerBooleanOption()             -269 -49.5%
  -0.0%    -480 llvm::APInt::toString()::Digits                               -480  -0.0%
 -67.4%    -496 clang::driver::toolchains::NetBSD::addLibCxxIncludePaths()    -496 -67.4%
  -0.0%    -576 [__TEXT,__cstring]                                            -576  -0.0%
  [ = ]       0 TOTAL                                                         -408  -0.0%

For codebases with more small arrays this likely plays better, but at least we know it's not a regression.

cfe/trunk/lib/CodeGen/CGDecl.cpp

 static llvm::Value *shouldUseMemSetToInitialize(llvm::Constant *Init,
                                                 uint64_t GlobalSize) {
   uint64_t SizeLimit = 32;
   if (GlobalSize <= SizeLimit)
     return nullptr;
   return llvm::isBytewiseValue(Init);
 }
 
-/// Decide whether we want to split a constant structure store into a sequence
-/// of its fields' stores. This may cost us code size and compilation speed,
-/// but plays better with store optimizations.
-static bool shouldSplitStructStore(CodeGenModule &CGM,
+/// Decide whether we want to split a constant structure or array store into a
+/// sequence of its fields' stores. This may cost us code size and compilation
+/// speed, but plays better with store optimizations.
+static bool shouldSplitConstantStore(CodeGenModule &CGM,
                                      uint64_t GlobalByteSize) {
-  // Don't break structures that occupy more than one cacheline.
+  // Don't break things that occupy more than one cacheline.
   uint64_t ByteSizeLimit = 64;
   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
     return false;
   if (GlobalByteSize <= ByteSizeLimit)
     return true;
   return false;
 }
 
static Address createUnnamedGlobalFrom(CodeGenModule &CGM, const VarDecl &D,
   return SrcPtr;
 }
 
 static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
                                   Address Loc, bool isVolatile,
                                   CGBuilderTy &Builder,
                                   llvm::Constant *constant) {
   auto *Ty = constant->getType();
-  bool isScalar = Ty->isIntOrIntVectorTy() || Ty->isPtrOrPtrVectorTy() ||
-                  Ty->isFPOrFPVectorTy();
-  if (isScalar) {
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+                          Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {
     Builder.CreateStore(constant, Loc, isVolatile);
     return;
   }
 
   auto *Int8Ty = llvm::IntegerType::getInt8Ty(CGM.getLLVMContext());
   auto *IntPtrTy = CGM.getDataLayout().getIntPtrType(CGM.getLLVMContext());
 
-  // If the initializer is all or mostly the same, codegen with bzero / memset
-  // then do a few stores afterward.
   uint64_t ConstantSize = CGM.getDataLayout().getTypeAllocSize(Ty);
   if (!ConstantSize)
     return;
   auto *SizeVal = llvm::ConstantInt::get(IntPtrTy, ConstantSize);
+
+  // If the initializer is all or mostly the same, codegen with bzero / memset
+  // then do a few stores afterward.
   if (shouldUseBZeroPlusStoresToInitialize(constant, ConstantSize)) {
     Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, 0), SizeVal,
                          isVolatile);
 
     bool valueAlreadyCorrect =
         constant->isNullValue() || isa<llvm::UndefValue>(constant);
     if (!valueAlreadyCorrect) {
       Loc = Builder.CreateBitCast(Loc, Ty->getPointerTo(Loc.getAddressSpace()));
       emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder);
     }
     return;
   }
 
+  // If the initializer is a repeated byte pattern, use memset.
   llvm::Value *Pattern = shouldUseMemSetToInitialize(constant, ConstantSize);
   if (Pattern) {
     uint64_t Value = 0x00;
     if (!isa<llvm::UndefValue>(Pattern)) {
       const llvm::APInt &AP = cast<llvm::ConstantInt>(Pattern)->getValue();
       assert(AP.getBitWidth() <= 8);
       Value = AP.getLimitedValue();
     }
     Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, Value), SizeVal,
                          isVolatile);
     return;
   }
 
-  llvm::StructType *STy = dyn_cast<llvm::StructType>(Ty);
+  // If the initializer is small, use a handful of stores.
+  if (shouldSplitConstantStore(CGM, ConstantSize)) {
+    if (auto *STy = dyn_cast<llvm::StructType>(Ty)) {
       // FIXME: handle the case when STy != Loc.getElementType().
-  // FIXME: handle non-struct aggregate types.
-  if (STy && (STy == Loc.getElementType()) &&
-      shouldSplitStructStore(CGM, ConstantSize)) {
+      if (STy == Loc.getElementType()) {
         for (unsigned i = 0; i != constant->getNumOperands(); i++) {
           Address EltPtr = Builder.CreateStructGEP(Loc, i);
           emitStoresForConstant(
               CGM, D, EltPtr, isVolatile, Builder,
               cast<llvm::Constant>(Builder.CreateExtractValue(constant, i)));
         }
         return;
       }
+    } else if (auto *ATy = dyn_cast<llvm::ArrayType>(Ty)) {
+      // FIXME: handle the case when ATy != Loc.getElementType().
+      if (ATy == Loc.getElementType()) {
+        for (unsigned i = 0; i != ATy->getNumElements(); i++) {
+          Address EltPtr = Builder.CreateConstArrayGEP(Loc, i);
+          emitStoresForConstant(
+              CGM, D, EltPtr, isVolatile, Builder,
+              cast<llvm::Constant>(Builder.CreateExtractValue(constant, i)));
+        }
+        return;
+      }
+    }
+  }
 
+  // Copy from a global.
   Builder.CreateMemCpy(
       Loc,
       createUnnamedGlobalFrom(CGM, D, Builder, constant, Loc.getAlignment()),
       SizeVal, isVolatile);
 }
 
 static void emitStoresForZeroInit(CodeGenModule &CGM, const VarDecl &D,
                                   Address Loc, bool isVolatile,

cfe/trunk/test/CodeGenCXX/auto-var-init.cpp

 
 // PATTERN-O0: @__const.test_bool4_uninit.uninit = private unnamed_addr constant [4 x i8] c"\AA\AA\AA\AA", align 1
 // PATTERN-O0: @__const.test_bool4_custom.custom = private unnamed_addr constant [4 x i8] c"\01\01\01\01", align 1
 // ZERO-O0: @__const.test_bool4_custom.custom = private unnamed_addr constant [4 x i8] c"\01\01\01\01", align 1
 // PATTERN-O1-NOT: @__const.test_bool4_uninit.uninit
 // PATTERN-O1-NOT: @__const.test_bool4_custom.custom
 // ZERO-O1-NOT: @__const.test_bool4_custom.custom
 
-// PATTERN: @__const.test_intptr4_uninit.uninit = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*)], align 16
 // PATTERN: @__const.test_intptr4_custom.custom = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*)], align 16
 // ZERO: @__const.test_intptr4_custom.custom = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*)], align 16
 // PATTERN-O0: @__const.test_tailpad4_uninit.uninit = private unnamed_addr constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }], align 16
 // PATTERN-O1-NOT: @__const.test_tailpad4_uninit.uninit
 // PATTERN: @__const.test_tailpad4_custom.custom = private unnamed_addr constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] c"\AA" }], align 16
 // ZERO: @__const.test_tailpad4_custom.custom = private unnamed_addr constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] zeroinitializer }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] zeroinitializer }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] zeroinitializer }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] zeroinitializer }], align 16
 struct tailpad { short s; char c; };
 // PATTERN-O0: @__const.test_atomicnotlockfree_uninit.uninit = private unnamed_addr constant %struct.notlockfree { [4 x i64] [i64 -6148914691236517206, i64 -6148914691236517206, i64 -6148914691236517206, i64 -6148914691236517206] }, align 8

 TEST_CUSTOM(bool4, bool[4], { true, true, true, true });
 // CHECK-LABEL: @test_bool4_custom()
 // CHECK:       %custom = alloca [4 x i8], align
 // CHECK-NEXT:  bitcast
 // CHECK-NEXT:  call void @llvm.memcpy
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
 TEST_UNINIT(intptr4, int*[4]);
-// CHECK-LABEL: @test_intptr4_uninit()
-// CHECK:       %uninit = alloca [4 x i32*], align
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
-// PATTERN-LABEL: @test_intptr4_uninit()
-// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit
-// ZERO-LABEL: @test_intptr4_uninit()
-// ZERO: call void @llvm.memset{{.*}}, i8 0,
+// CHECK-LABEL:      @test_intptr4_uninit()
+// CHECK:            %uninit = alloca [4 x i32*], align
+// CHECK-NEXT:       call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-O1-LABEL: @test_intptr4_uninit()
+// PATTERN-O1:       %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 0
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %1, align 16
+// PATTERN-O1-NEXT:  %2 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 1
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %2, align 8
+// PATTERN-O1-NEXT:  %3 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 2
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %3, align 16
+// PATTERN-O1-NEXT:  %4 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 3
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %4, align 8
+// ZERO-LABEL:       @test_intptr4_uninit()
+// ZERO:             call void @llvm.memset{{.*}}, i8 0,
 
 TEST_BRACES(intptr4, int*[4]);
 // CHECK-LABEL: @test_intptr4_braces()
 // CHECK:       %braces = alloca [4 x i32*], align [[ALIGN:[0-9]*]]
 // CHECK-NEXT:  bitcast
 // CHECK-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %{{.*}}, i8 0, i64 32, i1 false)
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 

 
 TEST_UNINIT(atomicnotlockfree, _Atomic(notlockfree));
 // CHECK-LABEL: @test_atomicnotlockfree_uninit()
 // CHECK:       %uninit = alloca %struct.notlockfree, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 // PATTERN-LABEL: @test_atomicnotlockfree_uninit()
 // PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_atomicnotlockfree_uninit.uninit
 // PATTERN-O1: bitcast
-// PATTERN-O1: call void @llvm.memset{{.*}}({{.*}}0, i8 -86, i64 32
+// PATTERN-O1: call void @llvm.memset{{.*}}({{.*}}, i8 -86, i64 32
 // ZERO-LABEL: @test_atomicnotlockfree_uninit()
 // ZERO: call void @llvm.memset{{.*}}, i8 0,
 
 TEST_UNINIT(atomicpadded, _Atomic(padded));
 // CHECK-LABEL: @test_atomicpadded_uninit()
 // CHECK:       %uninit = alloca %struct.padded, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 // PATTERN-LABEL: @test_atomicpadded_uninit()