vitalybuka updated this revision to Diff 208594.
vitalybuka added a comment.
undo some behavior change
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64382/new/
https://reviews.llvm.org/D64382
Files:
clang/lib/CodeGen/CGDecl.cpp
Index: clang/lib/CodeGen/CGDecl.cpp
===================================================================
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -860,52 +860,59 @@
EmitStoreOfScalar(value, lvalue, /* isInitialization */ true);
}
-/// 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,
- unsigned &NumStores) {
- // Zero and Undef never requires any extra stores.
- if (isa<llvm::ConstantAggregateZero>(Init) ||
- isa<llvm::ConstantPointerNull>(Init) ||
- isa<llvm::UndefValue>(Init))
+/// Decide whether we can emit the non-pattern parts of the specified
+/// initializer with equal or fewer than NumStores scalar stores.
+static bool canEmitStoresForInitAfterMemset(CodeGenModule &CGM,
+ llvm::Constant *Init,
+ llvm::Value *Pattern,
+ unsigned &NumStores) {
+ assert(Pattern);
+ llvm::Value *BWInit = isBytewiseValue(Init, CGM.getDataLayout());
+ if (BWInit && (BWInit == Pattern || isa<llvm::UndefValue>(BWInit)))
return true;
- if (isa<llvm::ConstantInt>(Init) || isa<llvm::ConstantFP>(Init) ||
+ if (BWInit || isa<llvm::ConstantInt>(Init) || isa<llvm::ConstantFP>(Init) ||
isa<llvm::ConstantVector>(Init) || isa<llvm::BlockAddress>(Init) ||
- isa<llvm::ConstantExpr>(Init))
- return Init->isNullValue() || NumStores--;
-
- // See if we can emit each element.
- if (isa<llvm::ConstantArray>(Init) || isa<llvm::ConstantStruct>(Init)) {
- for (unsigned i = 0, e = Init->getNumOperands(); i != e; ++i) {
- llvm::Constant *Elt = cast<llvm::Constant>(Init->getOperand(i));
- if (!canEmitInitWithFewStoresAfterBZero(Elt, NumStores))
- return false;
- }
- return true;
+ isa<llvm::ConstantExpr>(Init) || isa<llvm::GlobalValue>(Init)) {
+ // Single store or memset.
+ return NumStores--;
}
if (llvm::ConstantDataSequential *CDS =
- dyn_cast<llvm::ConstantDataSequential>(Init)) {
+ dyn_cast<llvm::ConstantDataSequential>(Init)) {
for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i) {
llvm::Constant *Elt = CDS->getElementAsConstant(i);
- if (!canEmitInitWithFewStoresAfterBZero(Elt, NumStores))
+ if (!canEmitStoresForInitAfterMemset(CGM, Elt, Pattern, NumStores))
return false;
}
return true;
}
+ for (unsigned i = 0, e = Init->getNumOperands(); i != e; ++i) {
+ llvm::Constant *Elt = cast<llvm::Constant>(Init->getOperand(i));
+ if (!canEmitStoresForInitAfterMemset(CGM, Elt, Pattern, NumStores))
+ return false;
+ return true;
+ }
+
// Anything else is hard and scary.
return false;
}
-/// For inits that canEmitInitWithFewStoresAfterBZero returned true for, emit
+/// For inits that canEmitStoresForInitAfterMemset 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) {
- assert(!Init->isNullValue() && !isa<llvm::UndefValue>(Init) &&
- "called emitStoresForInitAfterBZero for zero or undef value.");
-
+static void emitStoresForInitAfterMemset(CodeGenModule &CGM,
+ llvm::Constant *Init,
+ Address ParentLoc, int IdxInParent,
+ llvm::Value *Pattern, bool isVolatile,
+ CGBuilderTy &Builder) {
+ assert(Pattern);
+ llvm::Value *BWInit = isBytewiseValue(Init, CGM.getDataLayout());
+ if (BWInit && (BWInit == Pattern || isa<llvm::UndefValue>(BWInit)))
+ return;
+ Address Loc =
+ IdxInParent < 0
+ ? Builder.CreateElementBitCast(ParentLoc, Init->getType())
+ : Builder.CreateConstInBoundsGEP2_32(ParentLoc, 0, IdxInParent);
if (isa<llvm::ConstantInt>(Init) || isa<llvm::ConstantFP>(Init) ||
isa<llvm::ConstantVector>(Init) || isa<llvm::BlockAddress>(Init) ||
isa<llvm::ConstantExpr>(Init)) {
@@ -913,65 +920,78 @@
return;
}
+ if (BWInit) {
+ auto *SizeVal = llvm::ConstantInt::get(
+ CGM.IntPtrTy, CGM.getDataLayout().getTypeStoreSize(Init->getType()));
+ Builder.CreateMemSet(Loc, BWInit, SizeVal, isVolatile);
+ return;
+ }
+
if (llvm::ConstantDataSequential *CDS =
dyn_cast<llvm::ConstantDataSequential>(Init)) {
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))
- emitStoresForInitAfterBZero(
- CGM, Elt, Builder.CreateConstInBoundsGEP2_32(Loc, 0, i), isVolatile,
- Builder);
+ emitStoresForInitAfterMemset(CGM, Elt, Loc, i, Pattern, isVolatile,
+ Builder);
}
return;
}
-
assert((isa<llvm::ConstantStruct>(Init) || isa<llvm::ConstantArray>(Init)) &&
"Unknown value type!");
for (unsigned i = 0, e = Init->getNumOperands(); i != e; ++i) {
llvm::Constant *Elt = cast<llvm::Constant>(Init->getOperand(i));
-
- // 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);
+ emitStoresForInitAfterMemset(CGM, Elt, Loc, i, Pattern, isVolatile,
+ Builder);
}
}
-/// Decide whether we should use bzero plus some stores to initialize a local
-/// variable instead of using a memcpy from a constant global. It is beneficial
-/// to use bzero if the global is all zeros, or mostly zeros and large.
-static bool shouldUseBZeroPlusStoresToInitialize(llvm::Constant *Init,
- uint64_t GlobalSize) {
- // If a global is all zeros, always use a bzero.
- if (isa<llvm::ConstantAggregateZero>(Init)) return true;
-
- // If a non-zero global is <= 32 bytes, always use a memcpy. If it is large,
- // do it if it will require 6 or fewer scalar stores.
- // TODO: Should budget depends on the size? Avoiding a large global warrants
- // plopping in more stores.
- unsigned StoreBudget = 6;
- uint64_t SizeLimit = 32;
-
- return GlobalSize > SizeLimit &&
- canEmitInitWithFewStoresAfterBZero(Init, StoreBudget);
-}
-
/// Decide whether we should use memset to initialize a local variable instead
-/// of using a memcpy from a constant global. Assumes we've already decided to
-/// not user bzero.
-/// FIXME We could be more clever, as we are for bzero above, and generate
-/// memset followed by stores. It's unclear that's worth the effort.
-static llvm::Value *shouldUseMemSetToInitialize(llvm::Constant *Init,
- uint64_t GlobalSize,
- const llvm::DataLayout &DL) {
+/// of using a memcpy from a constant global.
+static llvm::Value *shouldUseMemSetToInitialize(CodeGenModule &CGM,
+ llvm::Constant *Init,
+ uint64_t GlobalSize) {
+ llvm::MostFrequentByte MFB =
+ llvm::getMostFrequentByte(Init, CGM.getDataLayout());
+
+ if (0) {
+ // TODO: Use this branch and remove 'else'. Now we can do that for any
+ // value, but it requires tests updates, so I'll enable it in a separate
+ // patch.
+ if (MFB.Value && !MFB.Other)
+ return MFB.Value;
+ } else {
+ if (isa<llvm::ConstantAggregateZero>(Init)) {
+ assert(MFB.Value);
+ assert(!MFB.Other);
+ return MFB.Value;
+ }
+ }
+
+ // TODO: Tweak these constants or use "density".
uint64_t SizeLimit = 32;
if (GlobalSize <= SizeLimit)
return nullptr;
- return llvm::isBytewiseValue(Init, DL);
+
+ // TODO: Remove. It's here just to keep behavior.
+ if (MFB.Other && MFB.Value &&
+ MFB.Value != llvm::ConstantInt::get(CGM.Int8Ty, 0))
+ return nullptr;
+
+ if (!MFB.Value) {
+ // We don't know most frequent byte, we may still completely initialize it
+ // with small number of stores.
+ // TODO: Use llvm::UndefValue::get. If we get here and later we decide
+ // that number of stores is small enough to use memset+store then store are
+ // going to replace entire bzeroed range. With UndefValue we can skip that
+ // bogus memset. I will do that as a separate patch.
+ MFB.Value = llvm::ConstantInt::get(CGM.Int8Ty, 0);
+ }
+
+ unsigned StoreBudget = 6;
+ return canEmitStoresForInitAfterMemset(CGM, Init, MFB.Value, StoreBudget)
+ ? MFB.Value
+ : nullptr;
}
/// Decide whether we want to split a constant structure or array store into a
@@ -1163,31 +1183,12 @@
// 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(CGM.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, CGM.getDataLayout());
- 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(CGM.Int8Ty, Value), SizeVal,
- isVolatile);
+ if (llvm::Value *Pattern =
+ shouldUseMemSetToInitialize(CGM, constant, ConstantSize)) {
+ if (!isa<llvm::UndefValue>(Pattern))
+ Builder.CreateMemSet(Loc, Pattern, SizeVal, isVolatile);
+ emitStoresForInitAfterMemset(CGM, constant, Loc, -1, Pattern, isVolatile,
+ Builder);
return;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
