[clang] [clang] Only set the trailing bytes to zero when filling a partially … (PR #79502)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 15 04:52:08 PST 2024
https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/79502
>From ead2aca22a0f964d0f316e5ea7a5e3967844c015 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Thu, 25 Jan 2024 22:12:55 +0100
Subject: [PATCH] [clang] Only set the trailing bytes to zero when filling a
partially initialized array
This patch leverages on the fact that padding bytes have an undefined
value, which changes the previous undefined behavior which previously was
zero in case of partial initialization.
Fix #79500
---
clang/lib/CodeGen/CGDecl.cpp | 98 +++++++++++++++----
.../test/CodeGenCXX/trivial-auto-var-init.cpp | 34 ++++++-
.../test/CodeGenOpenCL/partial_initializer.cl | 3 +-
3 files changed, 114 insertions(+), 21 deletions(-)
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index bbe14ef4c17244..6f7674c6b701eb 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -905,6 +905,11 @@ void CodeGenFunction::EmitScalarInit(const Expr *init, const ValueDecl *D,
EmitStoreOfScalar(value, lvalue, /* isInitialization */ true);
}
+static bool isNullOrUndef(llvm::Constant *C) {
+ return C->isNullValue() || isa<llvm::ConstantAggregateZero>(C) ||
+ isa<llvm::ConstantPointerNull>(C) || isa<llvm::UndefValue>(C);
+}
+
/// Decide whether we can emit the non-zero parts of the specified initializer
/// with equal or fewer than NumStores scalar stores.
static bool canEmitInitWithFewStoresAfterBZero(llvm::Constant *Init,
@@ -945,48 +950,89 @@ static bool canEmitInitWithFewStoresAfterBZero(llvm::Constant *Init,
/// For inits that canEmitInitWithFewStoresAfterBZero returned true for, emit
/// the scalar stores that would be required.
-static void emitStoresForInitAfterBZero(CodeGenModule &CGM,
- llvm::Constant *Init, Address Loc,
- bool isVolatile, CGBuilderTy &Builder,
- bool IsAutoInit) {
+static uint64_t emitStoresForInitAfterBZero(CodeGenModule &CGM,
+ llvm::Constant *Init, Address Loc,
+ bool isVolatile,
+ CGBuilderTy &Builder,
+ bool IsAutoInit) {
assert(!Init->isNullValue() && !isa<llvm::UndefValue>(Init) &&
"called emitStoresForInitAfterBZero for zero or undef value.");
+ auto const &DL = CGM.getDataLayout();
+
if (isa<llvm::ConstantInt>(Init) || isa<llvm::ConstantFP>(Init) ||
isa<llvm::ConstantVector>(Init) || isa<llvm::BlockAddress>(Init) ||
isa<llvm::ConstantExpr>(Init)) {
auto *I = Builder.CreateStore(Init, Loc, isVolatile);
if (IsAutoInit)
I->addAnnotationMetadata("auto-init");
- return;
+ return DL.getTypeAllocSize(Init->getType());
}
if (llvm::ConstantDataSequential *CDS =
dyn_cast<llvm::ConstantDataSequential>(Init)) {
+ bool CountNonNullBytes = true;
+ uint64_t LeadingNonNullElementsCount = 0;
for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i) {
llvm::Constant *Elt = CDS->getElementAsConstant(i);
// If necessary, get a pointer to the element and emit it.
- if (!Elt->isNullValue() && !isa<llvm::UndefValue>(Elt))
+ if (!isNullOrUndef(Elt)) {
emitStoresForInitAfterBZero(
CGM, Elt, Builder.CreateConstInBoundsGEP2_32(Loc, 0, i), isVolatile,
Builder, IsAutoInit);
+ if (CountNonNullBytes)
+ ++LeadingNonNullElementsCount;
+ } else if (CountNonNullBytes)
+ CountNonNullBytes = false;
}
- return;
+ uint64_t ElementByteCount = DL.getTypeAllocSize(CDS->getElementType());
+ return LeadingNonNullElementsCount * ElementByteCount;
}
assert((isa<llvm::ConstantStruct>(Init) || isa<llvm::ConstantArray>(Init)) &&
"Unknown value type!");
+ bool CountNonNullBytes = true;
+ uint64_t Offset = DL.getTypeAllocSize(Init->getType());
+
for (unsigned i = 0, e = Init->getNumOperands(); i != e; ++i) {
- llvm::Constant *Elt = cast<llvm::Constant>(Init->getOperand(i));
+ llvm::Constant *Operand = cast<llvm::Constant>(Init->getOperand(i));
+ uint64_t OperandByteCount = DL.getTypeAllocSize(Operand->getType());
+
+ uint64_t OperandOffset;
+ if (isNullOrUndef(Operand)) {
+ OperandOffset = 0;
+ } else {
+ // If necessary, get a pointer to the element and emit it.
+ OperandOffset = emitStoresForInitAfterBZero(
+ CGM, Operand, Builder.CreateConstInBoundsGEP2_32(Loc, 0, i),
+ isVolatile, Builder, IsAutoInit);
+ }
- // If necessary, get a pointer to the element and emit it.
- if (!Elt->isNullValue() && !isa<llvm::UndefValue>(Elt))
- emitStoresForInitAfterBZero(CGM, Elt,
- Builder.CreateConstInBoundsGEP2_32(Loc, 0, i),
- isVolatile, Builder, IsAutoInit);
+ if (CountNonNullBytes) {
+ if (OperandOffset != OperandByteCount) {
+ CountNonNullBytes = false;
+
+ // Add the offset of current field.
+ if (auto *CS = dyn_cast<llvm::ConstantStruct>(Init)) {
+ llvm::StructType *CST = CS->getType();
+ const llvm::StructLayout *SL = DL.getStructLayout(CST);
+ OperandOffset += SL->getElementOffset(i);
+ } else if (auto *CA = dyn_cast<llvm::ConstantArray>(Init)) {
+ llvm::ArrayType *CAT = CA->getType();
+ uint64_t ElementByteCount =
+ DL.getTypeAllocSize(CAT->getElementType());
+ OperandOffset += ElementByteCount * i;
+ } else {
+ assert(false && "unexpected constant type");
+ }
+
+ Offset = OperandOffset;
+ }
+ }
}
+ return Offset;
}
/// Decide whether we should use bzero plus some stores to initialize a local
@@ -1205,12 +1251,13 @@ static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
}
auto *SizeVal = llvm::ConstantInt::get(CGM.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)) {
- auto *I = Builder.CreateMemSet(Loc, llvm::ConstantInt::get(CGM.Int8Ty, 0),
- SizeVal, isVolatile);
+ auto *I = Builder.CreateMemSet(
+ Loc, llvm::ConstantInt::get(CGM.Int8Ty, 0),
+ llvm::ConstantInt::get(CGM.IntPtrTy, ConstantSize), isVolatile);
+
if (IsAutoInit)
I->addAnnotationMetadata("auto-init");
@@ -1218,8 +1265,23 @@ static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
constant->isNullValue() || isa<llvm::UndefValue>(constant);
if (!valueAlreadyCorrect) {
Loc = Loc.withElementType(Ty);
- emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder,
- IsAutoInit);
+ uint64_t Offset = emitStoresForInitAfterBZero(
+ CGM, constant, Loc, isVolatile, Builder, IsAutoInit);
+ // Prevent optimizing to odd offsets.
+ if (Offset) {
+ if (Offset == ConstantSize) {
+ I->eraseFromParent();
+ } else {
+ Loc = Loc.withElementType(CGM.Int8Ty);
+
+ llvm::Value *AdjustedLoc = llvm::GetElementPtrInst::CreateInBounds(
+ CGM.Int8Ty, Loc.getPointer(),
+ {llvm::ConstantInt::get(CGM.IntPtrTy, Offset)}, "", I);
+ I->setOperand(0, AdjustedLoc);
+ I->setOperand(
+ 2, llvm::ConstantInt::get(CGM.IntPtrTy, ConstantSize - Offset));
+ }
+ }
}
return;
}
diff --git a/clang/test/CodeGenCXX/trivial-auto-var-init.cpp b/clang/test/CodeGenCXX/trivial-auto-var-init.cpp
index eed9868cad07f8..e75e4d2ab31eb4 100644
--- a/clang/test/CodeGenCXX/trivial-auto-var-init.cpp
+++ b/clang/test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -288,14 +288,14 @@ void test_huge_uninit() {
// UNINIT-LABEL: test_huge_small_init(
// ZERO-LABEL: test_huge_small_init(
-// ZERO: call void @llvm.memset{{.*}}, i8 0, i64 65536,
+// ZERO: call void @llvm.memset{{.*}}, i8 0, i64 65532,
// ZERO-NOT: !annotation
// ZERO: store i8 97,
// ZERO: store i8 98,
// ZERO: store i8 99,
// ZERO: store i8 100,
// PATTERN-LABEL: test_huge_small_init(
-// PATTERN: call void @llvm.memset{{.*}}, i8 0, i64 65536,
+// PATTERN: call void @llvm.memset{{.*}}, i8 0, i64 65532,
// PATTERN-NOT: !annotation
// PATTERN: store i8 97,
// PATTERN: store i8 98,
@@ -318,6 +318,36 @@ void test_huge_larger_init() {
used(big);
}
+// UNINIT-LABEL: test_padded_struct_partial_init(
+// UNINIT: call void @llvm.memset{{.*}}, i8 0, i64 32, i1 false)
+// UNINIT: store i32 1,
+// UNINIT: store i8 97,
+// UNINIT: store i32 2,
+// UNINIT-NOT: !annotation
+// ZERO-LABEL: test_padded_struct_partial_init(
+// ZERO: call void @llvm.memset{{.*}}, i8 0, i64 39, i1 false)
+// ZERO: store i32 1,
+// ZERO: store i8 97,
+// ZERO: store i32 2,
+// ZERO-NOT: !annotation
+// PATTERN-LABEL: test_padded_struct_partial_init(
+// PATTERN: call void @llvm.memset{{.*}}, i8 0, i64 39, i1 false)
+// PATTERN: store i32 1,
+// PATTERN: store i8 97,
+// PATTERN: store i32 2,
+// PATTERN-NOT: !annotation
+struct padded {
+ int i0;
+ char c;
+ int i1;
+ int d[8];
+};
+padded test_padded_struct_partial_init() {
+ padded s = {1, 'a', 2,};
+ return s;
+}
+
+
} // extern "C"
// CHECK: [[AUTO_INIT]] = !{ !"auto-init" }
diff --git a/clang/test/CodeGenOpenCL/partial_initializer.cl b/clang/test/CodeGenOpenCL/partial_initializer.cl
index 5cc4e2b246003a..7c01c750d1afef 100644
--- a/clang/test/CodeGenOpenCL/partial_initializer.cl
+++ b/clang/test/CodeGenOpenCL/partial_initializer.cl
@@ -35,7 +35,8 @@ void f(void) {
// CHECK: %[[compoundliteral1:.*]] = alloca <2 x i32>, align 8
// CHECK: %[[V2:.*]] = alloca <4 x i32>, align 16
- // CHECK: call void @llvm.memset.p0.i32(ptr align 4 %A, i8 0, i32 144, i1 false)
+ // CHECK: %[[v0:.*]] = getelementptr inbounds i8, ptr %A, i32 8
+ // CHECK: call void @llvm.memset.p0.i32(ptr align 4 %[[v0]], i8 0, i32 136, i1 false)
// CHECK: %[[v2:.*]] = getelementptr inbounds [6 x [6 x float]], ptr %A, i32 0, i32 0
// CHECK: %[[v3:.*]] = getelementptr inbounds [6 x float], ptr %[[v2]], i32 0, i32 0
// CHECK: store float 1.000000e+00, ptr %[[v3]], align 4
More information about the cfe-commits
mailing list