Skip to content

Commit

Permalink
Refine generation of TBAA information in clang
Browse files Browse the repository at this point in the history 
This patch is an attempt to clarify and simplify generation and
propagation of TBAA information. The idea is to pack all values
that describe a memory access, namely, base type, access type and
offset, into a single structure. This is supposed to make further
changes, such as adding support for unions and array members,
easier to prepare and review.

DecorateInstructionWithTBAA() is no more responsible for
converting types to tags. These implicit conversions not only
complicate reading the code, but also suggest assigning scalar
access tags while we generally prefer full-size struct-path tags.

TBAAPathTag is replaced with TBAAAccessInfo; the latter is now
the type of the keys of the cache map that translates access
descriptors to metadata nodes.

Fixed a bug with writing to a wrong map in
getTBAABaseTypeMetadata() (former getTBAAStructTypeInfo()).

We now check for valid base access types every time we
dereference a field. The original code only checks the top-level
base type. See isValidBaseType() / isTBAAPathStruct() calls.

Some entities have been renamed to sound more adequate and less
confusing/misleading in presence of path-aware TBAA information.

Now we do not lookup twice for the same cache entry in
getAccessTagInfo().

Refined relevant comments and descriptions.

Differential Revision: https://reviews.llvm.org/D37826

llvm-svn: 315048
  • Loading branch information
Ivan A. Kosarev committed Oct 6, 2017
1 parent 468e2f6 commit 383890b
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 107 deletions.
8 changes: 3 additions & 5 deletions clang/lib/CodeGen/CGAtomic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ namespace {
addr = CGF.Builder.CreateStructGEP(addr, 0, CharUnits());

return LValue::MakeAddr(addr, getValueType(), CGF.getContext(),
LVal.getBaseInfo(), LVal.getTBAAAccessType());
LVal.getBaseInfo(), LVal.getTBAAInfo());
}

/// \brief Emits atomic load.
Expand Down Expand Up @@ -1425,8 +1425,7 @@ llvm::Value *AtomicInfo::EmitAtomicLoadOp(llvm::AtomicOrdering AO,
// Other decoration.
if (IsVolatile)
Load->setVolatile(true);
TBAAAccessInfo TBAAInfo(LVal.getTBAAAccessType());
CGF.CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAInfo());
return Load;
}

Expand Down Expand Up @@ -1942,8 +1941,7 @@ void CodeGenFunction::EmitAtomicStore(RValue rvalue, LValue dest,
// Other decoration.
if (IsVolatile)
store->setVolatile(true);
TBAAAccessInfo TBAAInfo(dest.getTBAAAccessType());
CGM.DecorateInstructionWithTBAA(store, TBAAInfo);
CGM.DecorateInstructionWithTBAA(store, dest.getTBAAInfo());
return;
}

Expand Down
63 changes: 35 additions & 28 deletions clang/lib/CodeGen/CGExpr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1174,8 +1174,7 @@ LValue CodeGenFunction::EmitLValue(const Expr *E) {
llvm::Value *V = LV.getPointer();
Scope.ForceCleanup({&V});
return LValue::MakeAddr(Address(V, LV.getAlignment()), LV.getType(),
getContext(), LV.getBaseInfo(),
LV.getTBAAAccessType());
getContext(), LV.getBaseInfo(), LV.getTBAAInfo());
}
// FIXME: Is it possible to create an ExprWithCleanups that produces a
// bitfield lvalue or some other non-simple lvalue?
Expand Down Expand Up @@ -1514,7 +1513,7 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool Volatile,

// Atomic operations have to be done on integral types.
LValue AtomicLValue =
LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
if (Ty->isAtomicType() || LValueIsSuitableForInlineAtomic(AtomicLValue)) {
return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
}
Expand All @@ -1525,11 +1524,10 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool Volatile,
Load->getContext(), llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
Load->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
}
if (TBAAInfo.AccessType) {
if (BaseInfo.getMayAlias())
TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
}

if (BaseInfo.getMayAlias())
TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);

if (EmitScalarRangeCheck(Load, Ty, Loc)) {
// In order to prevent the optimizer from throwing away the check, don't
Expand Down Expand Up @@ -1596,7 +1594,7 @@ void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, Address Addr,
Value = EmitToMemory(Value, Ty);

LValue AtomicLValue =
LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
if (Ty->isAtomicType() ||
(!isInit && LValueIsSuitableForInlineAtomic(AtomicLValue))) {
EmitAtomicStore(RValue::get(Value), AtomicLValue, isInit);
Expand All @@ -1610,11 +1608,10 @@ void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, Address Addr,
llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
Store->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
}
if (TBAAInfo.AccessType) {
if (BaseInfo.getMayAlias())
TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
}

if (BaseInfo.getMayAlias())
TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
}

void CodeGenFunction::EmitStoreOfScalar(llvm::Value *value, LValue lvalue,
Expand Down Expand Up @@ -3753,30 +3750,40 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,

LValue LV = MakeAddrLValue(addr, type, FieldBaseInfo);
LV.getQuals().addCVRQualifiers(cvr);
if (TBAAPath) {

// Fields of may_alias structs act like 'char' for TBAA purposes.
// FIXME: this should get propagated down through anonymous structs
// and unions.
if (mayAlias) {
LV.setTBAAInfo(CGM.getTBAAMayAliasAccessInfo());
} else if (TBAAPath) {
// If no base type been assigned for the base access, then try to generate
// one for this base lvalue.
TBAAAccessInfo TBAAInfo = base.getTBAAInfo();
if (!TBAAInfo.BaseType) {
TBAAInfo.BaseType = CGM.getTBAABaseTypeInfo(base.getType());
assert(!TBAAInfo.Offset &&
"Nonzero offset for an access with no base type!");
}

// Adjust offset to be relative to the base type.
const ASTRecordLayout &Layout =
getContext().getASTRecordLayout(field->getParent());
// Set the base type to be the base type of the base LValue and
// update offset to be relative to the base type.
unsigned CharWidth = getContext().getCharWidth();
TBAAAccessInfo TBAAInfo = mayAlias ?
CGM.getTBAAMayAliasAccessInfo() :
TBAAAccessInfo(base.getTBAAInfo().BaseType, CGM.getTBAATypeInfo(type),
base.getTBAAInfo().Offset + Layout.getFieldOffset(
field->getFieldIndex()) / CharWidth);
if (TBAAInfo.BaseType)
TBAAInfo.Offset +=
Layout.getFieldOffset(field->getFieldIndex()) / CharWidth;

// Update the final access type.
TBAAInfo.AccessType = LV.getTBAAInfo().AccessType;

LV.setTBAAInfo(TBAAInfo);
}

// __weak attribute on a field is ignored.
if (LV.getQuals().getObjCGCAttr() == Qualifiers::Weak)
LV.getQuals().removeObjCGCAttr();

// Fields of may_alias structs act like 'char' for TBAA purposes.
// FIXME: this should get propagated down through anonymous structs
// and unions.
if (mayAlias && LV.getTBAAAccessType())
LV.setTBAAInfo(CGM.getTBAAMayAliasAccessInfo());

return LV;
}

Expand Down
15 changes: 5 additions & 10 deletions clang/lib/CodeGen/CGValue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class LValue {
private:
void Initialize(QualType Type, Qualifiers Quals,
CharUnits Alignment, LValueBaseInfo BaseInfo,
llvm::MDNode *TBAAAccessType = nullptr) {
TBAAAccessInfo TBAAInfo = TBAAAccessInfo()) {
assert((!Alignment.isZero() || Type->isIncompleteType()) &&
"initializing l-value with zero alignment!");
this->Type = Type;
Expand All @@ -241,7 +241,7 @@ class LValue {
assert(this->Alignment == Alignment.getQuantity() &&
"Alignment exceeds allowed max!");
this->BaseInfo = BaseInfo;
this->TBAAInfo = TBAAAccessInfo(Type, TBAAAccessType, /* Offset= */ 0);
this->TBAAInfo = TBAAInfo;

// Initialize Objective-C flags.
this->Ivar = this->ObjIsArray = this->NonGC = this->GlobalObjCRef = false;
Expand Down Expand Up @@ -311,9 +311,6 @@ class LValue {
TBAAAccessInfo getTBAAInfo() const { return TBAAInfo; }
void setTBAAInfo(TBAAAccessInfo Info) { TBAAInfo = Info; }

llvm::MDNode *getTBAAAccessType() const { return TBAAInfo.AccessType; }
void setTBAAAccessType(llvm::MDNode *N) { TBAAInfo.AccessType = N; }

const Qualifiers &getQuals() const { return Quals; }
Qualifiers &getQuals() { return Quals; }

Expand Down Expand Up @@ -370,18 +367,16 @@ class LValue {
// global register lvalue
llvm::Value *getGlobalReg() const { assert(isGlobalReg()); return V; }

static LValue MakeAddr(Address address, QualType type,
ASTContext &Context,
LValueBaseInfo BaseInfo,
llvm::MDNode *TBAAAccessType = nullptr) {
static LValue MakeAddr(Address address, QualType type, ASTContext &Context,
LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
Qualifiers qs = type.getQualifiers();
qs.setObjCGCAttr(Context.getObjCGCAttrKind(type));

LValue R;
R.LVType = Simple;
assert(address.getPointer()->getType()->isPointerTy());
R.V = address.getPointer();
R.Initialize(type, qs, address.getAlignment(), BaseInfo, TBAAAccessType);
R.Initialize(type, qs, address.getAlignment(), BaseInfo, TBAAInfo);
return R;
}

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/CodeGenFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ LValue CodeGenFunction::MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T) {
LValueBaseInfo BaseInfo;
CharUnits Alignment = getNaturalTypeAlignment(T, &BaseInfo);
return LValue::MakeAddr(Address(V, Alignment), T, getContext(), BaseInfo,
CGM.getTBAATypeInfo(T));
CGM.getTBAAAccessInfo(T));
}

/// Given a value of type T* that may not be to a complete object,
Expand Down
22 changes: 18 additions & 4 deletions clang/lib/CodeGen/CodeGenFunction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1914,14 +1914,14 @@ class CodeGenFunction : public CodeGenTypeCache {
LValueBaseInfo BaseInfo =
LValueBaseInfo(AlignmentSource::Type)) {
return LValue::MakeAddr(Addr, T, getContext(), BaseInfo,
CGM.getTBAATypeInfo(T));
CGM.getTBAAAccessInfo(T));
}

LValue MakeAddrLValue(llvm::Value *V, QualType T, CharUnits Alignment,
LValueBaseInfo BaseInfo =
LValueBaseInfo(AlignmentSource::Type)) {
return LValue::MakeAddr(Address(V, Alignment), T, getContext(),
BaseInfo, CGM.getTBAATypeInfo(T));
BaseInfo, CGM.getTBAAAccessInfo(T));
}

LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
Expand Down Expand Up @@ -3060,7 +3060,14 @@ class CodeGenFunction : public CodeGenTypeCache {
SourceLocation Loc,
LValueBaseInfo BaseInfo =
LValueBaseInfo(AlignmentSource::Type),
TBAAAccessInfo TBAAInfo = TBAAAccessInfo(),
bool isNontemporal = false) {
return EmitLoadOfScalar(Addr, Volatile, Ty, Loc, BaseInfo,
CGM.getTBAAAccessInfo(Ty), isNontemporal);
}

llvm::Value *EmitLoadOfScalar(Address Addr, bool Volatile, QualType Ty,
SourceLocation Loc, LValueBaseInfo BaseInfo,
TBAAAccessInfo TBAAInfo,
bool isNontemporal = false);

/// EmitLoadOfScalar - Load a scalar value from an address, taking
Expand All @@ -3076,7 +3083,14 @@ class CodeGenFunction : public CodeGenTypeCache {
bool Volatile, QualType Ty,
LValueBaseInfo BaseInfo =
LValueBaseInfo(AlignmentSource::Type),
TBAAAccessInfo TBAAInfo = TBAAAccessInfo(),
bool isInit = false, bool isNontemporal = false) {
EmitStoreOfScalar(Value, Addr, Volatile, Ty, BaseInfo,
CGM.getTBAAAccessInfo(Ty), isInit, isNontemporal);
}

void EmitStoreOfScalar(llvm::Value *Value, Address Addr,
bool Volatile, QualType Ty,
LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo,
bool isInit = false, bool isNontemporal = false);

/// EmitStoreOfScalar - Store a scalar value to an address, taking
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,12 @@ llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType QTy) {
return TBAA->getTBAAStructInfo(QTy);
}

llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
if (!TBAA)
return nullptr;
return TBAA->getBaseTypeInfo(QTy);
}

llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
if (!TBAA)
return nullptr;
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,10 @@ class CodeGenModule : public CodeGenTypeCache {

llvm::MDNode *getTBAAStructInfo(QualType QTy);

/// getTBAABaseTypeInfo - Get metadata that describes the given base access
/// type. Return null if the type is not suitable for use in TBAA access tags.
llvm::MDNode *getTBAABaseTypeInfo(QualType QTy);

/// getTBAAAccessTagInfo - Get TBAA tag for a given memory access.
llvm::MDNode *getTBAAAccessTagInfo(TBAAAccessInfo Info);

Expand Down
38 changes: 15 additions & 23 deletions clang/lib/CodeGen/CodeGenTBAA.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ CodeGenTBAA::getTBAAStructInfo(QualType QTy) {

/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
if (QTy == QualType())
return false;
if (const RecordType *TTy = QTy->getAs<RecordType>()) {
const RecordDecl *RD = TTy->getDecl()->getDefinition();
if (RD->hasFlexibleArrayMember())
Expand All @@ -249,10 +247,11 @@ static bool isValidBaseType(QualType QTy) {
}

llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
assert(isValidBaseType(QTy));
if (!isValidBaseType(QTy))
return nullptr;

if (llvm::MDNode *N = StructTypeMetadataCache[Ty])
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
return N;

if (const RecordType *TTy = QTy->getAs<RecordType>()) {
Expand All @@ -267,7 +266,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
llvm::MDNode *FieldNode = isValidBaseType(FieldQTy) ?
getBaseTypeInfo(FieldQTy) : getTypeInfo(FieldQTy);
if (!FieldNode)
return StructTypeMetadataCache[Ty] = nullptr;
return BaseTypeMetadataCache[Ty] = nullptr;
Fields.push_back(std::make_pair(
FieldNode, Layout.getFieldOffset(idx) / Context.getCharWidth()));
}
Expand All @@ -281,11 +280,11 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
OutName = RD->getName();
}
// Create the struct type node with a vector of pairs (offset, type).
return StructTypeMetadataCache[Ty] =
return BaseTypeMetadataCache[Ty] =
MDHelper.createTBAAStructTypeNode(OutName, Fields);
}

return StructMetadataCache[Ty] = nullptr;
return BaseTypeMetadataCache[Ty] = nullptr;
}

llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
Expand All @@ -295,23 +294,16 @@ llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
if (!CodeGenOpts.StructPathTBAA)
Info = TBAAAccessInfo(Info.AccessType);

const Type *BTy = nullptr;
if (Info.BaseType != QualType())
BTy = Context.getCanonicalType(Info.BaseType).getTypePtr();
TBAAPathTag PathTag = TBAAPathTag(BTy, Info.AccessType, Info.Offset);
if (llvm::MDNode *N = StructTagMetadataCache[PathTag])
llvm::MDNode *&N = AccessTagMetadataCache[Info];
if (N)
return N;

llvm::MDNode *BNode = nullptr;
if (isValidBaseType(Info.BaseType))
BNode = getBaseTypeInfo(Info.BaseType);
if (!BNode)
return StructTagMetadataCache[PathTag] =
MDHelper.createTBAAStructTagNode(Info.AccessType, Info.AccessType,
/* Offset= */ 0);

return StructTagMetadataCache[PathTag] =
MDHelper.createTBAAStructTagNode(BNode, Info.AccessType, Info.Offset);
if (!Info.BaseType) {
Info.BaseType = Info.AccessType;
assert(!Info.Offset && "Nonzero offset for an access with no base type!");
}
return N = MDHelper.createTBAAStructTagNode(Info.BaseType, Info.AccessType,
Info.Offset);
}

TBAAAccessInfo CodeGenTBAA::getMayAliasAccessInfo() {
Expand Down
Loading

0 comments on commit 383890b

Please sign in to comment.