This is an archive of the discontinued LLVM Phabricator instance.

Paths

Table of Contentst

-
cfe/trunk/lib/CodeGen/
-
trunk/
-
lib/
-
CodeGen/
-
CGDecl.cpp

Differential D51752

NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue
ClosedPublic

Authored by jfb on Sep 6 2018, 2:09 PM.

Download Raw Diff

Details

Reviewers

MatzeB

Commits

rG2f0582fcc73f: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue
rC342734: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue
rL342734: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue

Summary

This code was in CGDecl.cpp and really belongs in LLVM. It happened to have isBytewiseValue which served a very similar purpose but wasn't as powerful as clang's version. Remove the clang version, and augment isBytewiseValue to be as powerful so that clang does the same thing it used to.

LLVM part of this patch: D51751

Diff Detail

Repository: rL LLVM

Event Timeline

jfb created this revision.Sep 6 2018, 2:09 PM

Herald added subscribers: cfe-commits, dexonsmith. · View Herald TranscriptSep 6 2018, 2:09 PM

Harbormaster completed remote builds in B22344: Diff 164287.Sep 6 2018, 2:09 PM

jfb mentioned this in D51751: Merge clang's isRepeatedBytePattern with LLVM's isBytewiseValue.Sep 6 2018, 2:10 PM

jfb added a reviewer: MatzeB.

I'm not a good person to review clang changes.
I assume the review lacks the changes on the llvm side?
On the LLVM side this feels like something for Analysis/ValueTracking.h in fact I already see Value *isBytewiseValue(Value *V); there, maybe that is good enough for your purpose?

In D51752#1226462, @MatzeB wrote:

I'm not a good person to review clang changes.

I assume the review lacks the changes on the llvm side?

On the LLVM side this feels like something for Analysis/ValueTracking.h in fact I already see Value *isBytewiseValue(Value *V); there, maybe that is good enough for your purpose?

Indeed, I just noticed the same. I'll try to figure out why it isn't firing where I want it to.

In D51752#1226465, @jfb wrote:

In D51752#1226462, @MatzeB wrote:

I'm not a good person to review clang changes.

I assume the review lacks the changes on the llvm side?

On the LLVM side this feels like something for Analysis/ValueTracking.h in fact I already see Value *isBytewiseValue(Value *V); there, maybe that is good enough for your purpose?

Indeed, I just noticed the same. I'll try to figure out why it isn't firing where I want it to.

I think I'll just improve isBytewiseValue to do all the things isRepeatedBytePattern does. I'll update this patch to still delete constantIsRepeatedBytePattern, but use the new and improved isBytewiseValue.

jfb planned changes to this revision.Sep 6 2018, 2:25 PM

jfb added a parent revision: D51751: Merge clang's isRepeatedBytePattern with LLVM's isBytewiseValue.

In D51752#1226462, @MatzeB wrote:

I assume the review lacks the changes on the llvm side?

Oops missed the link to the llvm changes in the description... At least in theory you should be able to make a review over all changes in parallel by using subversion or the experimental llvm git monorepo with llvm/utils/git-svn/git-llvm (admittedly haven't tried it myself yet either).

In D51752#1226474, @MatzeB wrote:

In D51752#1226462, @MatzeB wrote:

I assume the review lacks the changes on the llvm side?

Oops missed the link to the llvm changes in the description... At least in theory you should be able to make a review over all changes in parallel by using subversion or the experimental llvm git monorepo with llvm/utils/git-svn/git-llvm (admittedly haven't tried it myself yet either).

Haha yeah I'm still using my now years-old workflow. Haven't tried the new fancy ones yet, waiting for the actual move to do so :)

Use isBytewiseValue instead.

Harbormaster completed remote builds in B22380: Diff 164481.Sep 7 2018, 11:45 AM

jfb retitled this revision from NFC: move isRepeatedBytePattern from clang to Constant to NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue.Sep 7 2018, 11:47 AM

jfb edited the summary of this revision. (Show Details)

Looks obvious and LGTM.

This revision is now accepted and ready to land.Sep 7 2018, 1:14 PM

Will wait on D51751 before committing, otherwise clang tests will start failing (because current isRepeatedBytePattern isn't capable enough).

jfb mentioned this in rL342709: Merge clang's isRepeatedBytePattern with LLVM's isBytewiseValue.Sep 20 2018, 10:22 PM

Closed by commit rL342734: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue (authored by jfb). · Explain WhySep 21 2018, 6:55 AM

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: llvm-commits. · View Herald TranscriptSep 21 2018, 6:55 AM

Revision Contents

Path

Size

cfe/

trunk/

lib/

CodeGen/

CGDecl.cpp

114 lines

Diff 166472

cfe/trunk/lib/CodeGen/CGDecl.cpp

Show All 24 Lines
#include "clang/AST/CharUnits.h"		#include "clang/AST/CharUnits.h"
#include "clang/AST/Decl.h"		#include "clang/AST/Decl.h"
#include "clang/AST/DeclObjC.h"		#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclOpenMP.h"		#include "clang/AST/DeclOpenMP.h"
#include "clang/Basic/SourceManager.h"		#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"		#include "clang/Basic/TargetInfo.h"
#include "clang/CodeGen/CGFunctionInfo.h"		#include "clang/CodeGen/CGFunctionInfo.h"
#include "clang/Frontend/CodeGenOptions.h"		#include "clang/Frontend/CodeGenOptions.h"
		#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/DataLayout.h"		#include "llvm/IR/DataLayout.h"
#include "llvm/IR/GlobalVariable.h"		#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Intrinsics.h"		#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/Type.h"		#include "llvm/IR/Type.h"

using namespace clang;		using namespace clang;
using namespace CodeGen;		using namespace CodeGen;

▲ Show 20 Lines • Show All 902 Lines • ▼ Show 20 Lines	static bool shouldUseBZeroPlusStoresToInitialize(llvm::Constant *Init,
// plopping in more stores.		// plopping in more stores.
unsigned StoreBudget = 6;		unsigned StoreBudget = 6;
uint64_t SizeLimit = 32;		uint64_t SizeLimit = 32;

return GlobalSize > SizeLimit &&		return GlobalSize > SizeLimit &&
canEmitInitWithFewStoresAfterBZero(Init, StoreBudget);		canEmitInitWithFewStoresAfterBZero(Init, StoreBudget);
}		}

/// A byte pattern.
///
/// Can be "any" pattern if the value was padding or known to be undef.
/// Can be "none" pattern if a sequence doesn't exist.
class BytePattern {
uint8_t Val;
enum class ValueType : uint8_t { Specific, Any, None } Type;
BytePattern(ValueType Type) : Type(Type) {}

public:
BytePattern(uint8_t Value) : Val(Value), Type(ValueType::Specific) {}
static BytePattern Any() { return BytePattern(ValueType::Any); }
static BytePattern None() { return BytePattern(ValueType::None); }
bool isAny() const { return Type == ValueType::Any; }
bool isNone() const { return Type == ValueType::None; }
bool isValued() const { return Type == ValueType::Specific; }
uint8_t getValue() const {
assert(isValued());
return Val;
}
BytePattern merge(const BytePattern Other) const {
if (isNone() \|\| Other.isNone())
return None();
if (isAny())
return Other;
if (Other.isAny())
return *this;
if (getValue() == Other.getValue())
return *this;
return None();
}
};

/// Figures out whether the constant can be initialized with memset.
static BytePattern constantIsRepeatedBytePattern(llvm::Constant *C) {
if (isa<llvm::ConstantAggregateZero>(C) \|\| isa<llvm::ConstantPointerNull>(C))
return BytePattern(0x00);
if (isa<llvm::UndefValue>(C))
return BytePattern::Any();

if (isa<llvm::ConstantInt>(C)) {
auto *Int = cast<llvm::ConstantInt>(C);
if (Int->getBitWidth() % 8 != 0)
return BytePattern::None();
const llvm::APInt &Value = Int->getValue();
if (Value.isSplat(8))
return BytePattern(Value.getLoBits(8).getLimitedValue());
return BytePattern::None();
}

if (isa<llvm::ConstantFP>(C)) {
auto *FP = cast<llvm::ConstantFP>(C);
llvm::APInt Bits = FP->getValueAPF().bitcastToAPInt();
if (Bits.getBitWidth() % 8 != 0)
return BytePattern::None();
if (!Bits.isSplat(8))
return BytePattern::None();
return BytePattern(Bits.getLimitedValue() & 0xFF);
}

if (isa<llvm::ConstantVector>(C)) {
llvm::Constant *Splat = cast<llvm::ConstantVector>(C)->getSplatValue();
if (Splat)
return constantIsRepeatedBytePattern(Splat);
return BytePattern::None();
}

if (isa<llvm::ConstantArray>(C) \|\| isa<llvm::ConstantStruct>(C)) {
BytePattern Pattern(BytePattern::Any());
for (unsigned I = 0, E = C->getNumOperands(); I != E; ++I) {
llvm::Constant *Elt = cast<llvm::Constant>(C->getOperand(I));
Pattern = Pattern.merge(constantIsRepeatedBytePattern(Elt));
if (Pattern.isNone())
return Pattern;
}
return Pattern;
}

if (llvm::ConstantDataSequential *CDS =
dyn_cast<llvm::ConstantDataSequential>(C)) {
BytePattern Pattern(BytePattern::Any());
for (unsigned I = 0, E = CDS->getNumElements(); I != E; ++I) {
llvm::Constant *Elt = CDS->getElementAsConstant(I);
Pattern = Pattern.merge(constantIsRepeatedBytePattern(Elt));
if (Pattern.isNone())
return Pattern;
}
return Pattern;
}

// BlockAddress, ConstantExpr, and everything else is scary.
return BytePattern::None();
}

/// Decide whether we should use memset to initialize a local variable instead		/// 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		/// of using a memcpy from a constant global. Assumes we've already decided to
/// not user bzero.		/// not user bzero.
/// FIXME We could be more clever, as we are for bzero above, and generate		/// 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.		/// memset followed by stores. It's unclear that's worth the effort.
static BytePattern shouldUseMemSetToInitialize(llvm::Constant *Init,		static llvm::Value shouldUseMemSetToInitialize(llvm::Constant Init,
uint64_t GlobalSize) {		uint64_t GlobalSize) {
uint64_t SizeLimit = 32;		uint64_t SizeLimit = 32;
if (GlobalSize <= SizeLimit)		if (GlobalSize <= SizeLimit)
return BytePattern::None();		return nullptr;
return constantIsRepeatedBytePattern(Init);		return llvm::isBytewiseValue(Init);
}		}

static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,		static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
Address Loc, bool isVolatile,		Address Loc, bool isVolatile,
CGBuilderTy &Builder,		CGBuilderTy &Builder,
llvm::Constant *constant) {		llvm::Constant *constant) {
auto *Int8Ty = llvm::IntegerType::getInt8Ty(CGM.getLLVMContext());		auto *Int8Ty = llvm::IntegerType::getInt8Ty(CGM.getLLVMContext());
auto *IntPtrTy = CGM.getDataLayout().getIntPtrType(CGM.getLLVMContext());		auto *IntPtrTy = CGM.getDataLayout().getIntPtrType(CGM.getLLVMContext());
Show All 12 Lines	if (shouldUseBZeroPlusStoresToInitialize(constant, ConstantSize)) {
if (!valueAlreadyCorrect) {		if (!valueAlreadyCorrect) {
Loc = Builder.CreateBitCast(		Loc = Builder.CreateBitCast(
Loc, constant->getType()->getPointerTo(Loc.getAddressSpace()));		Loc, constant->getType()->getPointerTo(Loc.getAddressSpace()));
emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder);		emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder);
}		}
return;		return;
}		}

BytePattern Pattern = shouldUseMemSetToInitialize(constant, ConstantSize);		llvm::Value *Pattern = shouldUseMemSetToInitialize(constant, ConstantSize);
if (!Pattern.isNone()) {		if (Pattern) {
uint8_t Value = Pattern.isAny() ? 0x00 : Pattern.getValue();		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,		Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, Value), SizeVal,
isVolatile);		isVolatile);
return;		return;
}		}

// Otherwise, create a temporary global with the initializer then memcpy from		// Otherwise, create a temporary global with the initializer then memcpy from
// the global to the alloca.		// the global to the alloca.
std::string Name = getStaticDeclName(CGM, D);		std::string Name = getStaticDeclName(CGM, D);
▲ Show 20 Lines • Show All 1,142 Lines • Show Last 20 Lines