Refine generation of TBAA information in clang

[kosarev]

Summary

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.

[kosarev]

• 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.
• Removed unwanted stylistic changes.

[hfinkel]

lib/CodeGen/CGAtomic.cpp

1429 ↗

(On Diff #115183)

You have a number of changes like this. AccessType is already a MDNode*, and I'm not sure why you're doing this. Will TBAA always equal LVal.getTBAAInfo().AccessType?

[kosarev]

lib/CodeGen/CGAtomic.cpp

1429 ↗

(On Diff #115183)

Nope, it's not always the final access type that we want as the TBAA information.

Note that this snippet is functionally equivalent to the original piece. The difference is that the original code looks like we propagate the complete TBAA information (primarily of the misleading/outdated naming) while the patch makes it visible that we in fact decorate the atomic accesses with scalar tags that do not include any struct-path information. This is one of the reasons why the patched version looks more verbose. I am about to public an update to this patch that addresses this verbosity among other things.

[kosarev]

• TBAAAccessInfo is now capable of representing both scalar and struct-path accesses as well as the special cases like may-alias nodes and accesses to virtual table pointers.
• Simplified the code that decorates instructions with TBAA information.
• 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.
• Removed the scalar-specific cache map and related functions.
• Decoration of atomic accesses is fixed to propagate the complete TBAA information and not just the final access types.
• 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 isValidTBAABaseType() / isTBAAPathStruct() calls.
• Some entities have been renamed to sound more adequate and less confusing/misleading in presence of path-aware TBAA information.
• Added description of the TBAAAccessInfo invariant.
• Refined relevant comments and descriptions.

[kosarev]

lib/CodeGen/CodeGenTBAA.cpp

291 ↗

(On Diff #115399)

Note the writing to StructMetadataCache instead of StructTypeMetadataCache. This is fixed in the patched version.

[hfinkel]

lib/CodeGen/CGExpr.cpp

3739 ↗

(On Diff #115399)

This code has no dependence on mayAlias. Should it (if not for correctness, to avoid unnecessary work)? Maybe we should make the condition above:

if (TBAAPath && !mayAlias)

[kosarev]

lib/CodeGen/CGExpr.cpp

3739 ↗

(On Diff #115399)

Right, we better handle the mayAlias case in a special way so we do not generate inappropriate access descriptors, even temporarily. And save some execution time too. Thanks Hal!

[kosarev]

• Handle may-alias base lvalues in a special way to address the note from Hal; see EmitLoadOfLValue().
• Do not lookup twice for the same cache entry in getTBAAAccessMetadata().
• Declare the single-argument TBAAAccessInfo constructor to be explicit to make the type-to-access conversions visible.

[kosarev]

There are more TBAA patches on top of this change, of which the first is to make TBAA information to be part of LValueBaseInfo--all are steps toward the support for unions and member arrays. So friendly ping. Thanks.

[kosarev]

Colleagues, please let me know if I can do anything else to help with reviewing the patch. Thanks.

[rjmccall]

I assume I should wait on reviewing this until all of these smaller TBAA patches land?

[kosarev]

Rebased.

This is how how the rebased patch differs from the mainline:

• It incorporates changes from (not landed yet) D38503.
• Simplifies generation of TBAA info in EmitLValueForField().
• Replaces TBAAPathTag with TBAAAccessInfo for caching purposes.
• Decorated atomic accesses with complete TBAA info and not just final access types (propagation of TBAA info for atomic accesses is already fixed in D38460).
• Fixes the bug with writing to a wrong cache in getTBAAStructTypeInfo() / getBaseTypeInfo().

[kosarev]

In D37826#887820, @rjmccall wrote:

I assume I should wait on reviewing this until all of these smaller TBAA patches land?

These small patches are actually part of this diff. Generally, It depends on how you would like it: if you can review this re-based version as a whole, then that would save us some time.

Otherwise, you may want to proceed by smaller pieces, in which case D38503 is the next proposed piece to review.

Alternatively, if D38503 looks too complicated, please let me know and I will break it down into even smaller patches.

Or, if acceptable, I could commit a series of really small and simple diffs that all result in what is presented in D38126 and rely on post-commit reviews.

I'm fine with either way, provided the changes go to the mainline in time manner.

Thanks a lot for reviewing!

[rjmccall]

In D37826#888086, @kosarev wrote:

In D37826#887820, @rjmccall wrote:

I assume I should wait on reviewing this until all of these smaller TBAA patches land?

These small patches are actually part of this diff. Generally, It depends on how you would like it: if you can review this re-based version as a whole, then that would save us some time.

Otherwise, you may want to proceed by smaller pieces, in which case D38503 is the next proposed piece to review.

Alternatively, if D38503 looks too complicated, please let me know and I will break it down into even smaller patches.

Or, if acceptable, I could commit a series of really small and simple diffs that all result in what is presented in D38126 and rely on post-commit reviews.

I'm fine with either way, provided the changes go to the mainline in time manner.

Thanks a lot for reviewing!

I'll let you rebase this on top of the D38503 when you commit it, and then I'll just review this directly. Thanks a lot for splitting it up!

John.

[kosarev]

• Rebased on top of D38503.
• The tbaa-for-vptr.cpp test is changed to be more specific about the instructions to test. Otherwise, we fail on some triples, such as s390x-ibm-linux and x86_64-pc-mingw32. This is due to the changes for EmitLoadOfScalar() and EmitStoreOfScalar() where we previously were defaulting to empty TBAA info and now generate a TBAA descriptor based on the specified access type. This results in the instructions that load the pointer-to-function value having their TBAA tags (the may-alias one, namely) so that these instructions match the pattern in the test file and NUM gets a wrong value.

[kosarev]

lib/CodeGen/CodeGenTBAA.h

129 ↗

(On Diff #117823)

Wrong function name. Will fix on next review cycle or commit.

[rjmccall]

LGTM.

cfe/trunk/lib/CodeGen/CGAtomic.cpp

public:
     /// Project an l-value down to the value field.
     LValue projectValue() const {
       assert(LVal.isSimple());
       Address addr = getAtomicAddress();
       if (hasPadding())
         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.
     /// \returns Loaded value.
     RValue EmitAtomicLoad(AggValueSlot ResultSlot, SourceLocation Loc,
                           bool AsValue, llvm::AtomicOrdering AO,
                           bool IsVolatile);
 
llvm::Value *AtomicInfo::EmitAtomicLoadOp(llvm::AtomicOrdering AO,
   // Okay, we're doing this natively.
   Address Addr = getAtomicAddressAsAtomicIntPointer();
   llvm::LoadInst *Load = CGF.Builder.CreateLoad(Addr, "atomic-load");
   Load->setAtomic(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;
 }
 
 /// An LValue is a candidate for having its loads and stores be made atomic if
 /// we are operating under /volatile:ms *and* the LValue itself is volatile and
 /// performing such an operation can be performed without a libcall.
 bool CodeGenFunction::LValueIsSuitableForInlineAtomic(LValue LV) {
   if (!CGM.getCodeGenOpts().MSVolatile) return false;
if (LVal.isSimple()) {
 
     // Initializations don't need to be atomic.
     if (!isInit)
       store->setAtomic(AO);
 
     // Other decoration.
     if (IsVolatile)
       store->setVolatile(true);
-    TBAAAccessInfo TBAAInfo(dest.getTBAAAccessType());
-    CGM.DecorateInstructionWithTBAA(store, TBAAInfo);
+    CGM.DecorateInstructionWithTBAA(store, dest.getTBAAInfo());
     return;
   }
 
   // Emit simple atomic update operation.
   atomics.EmitAtomicUpdate(AO, rvalue, IsVolatile);
 }
 
 /// Emit a compare-and-exchange op for atomic type.

cfe/trunk/lib/CodeGen/CGExpr.cpp

case Expr::ExprWithCleanupsClass: {
     RunCleanupsScope Scope(*this);
     LValue LV = EmitLValue(cleanups->getSubExpr());
     if (LV.isSimple()) {
       // Defend against branches out of gnu statement expressions surrounded by
       // cleanups.
       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?
     return LV;
   }
 
   case Expr::CXXDefaultArgExprClass:
     return EmitLValue(cast<CXXDefaultArgExpr>(E)->getExpr());
if (Ty->isVectorType()) {
                                         {0, 1, 2}, "extractVec");
         return EmitFromMemory(V, Ty);
       }
     }
   }
 
   // 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();
   }
 
   llvm::LoadInst *Load = Builder.CreateLoad(Addr, Volatile);
   if (isNontemporal) {
     llvm::MDNode *Node = llvm::MDNode::get(
         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 (EmitScalarRangeCheck(Load, Ty, Loc)) {
     // In order to prevent the optimizer from throwing away the check, don't
     // attach range metadata to the load.
   } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
     if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
       Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
 
if (Ty->isVectorType()) {
         Addr = Builder.CreateElementBitCast(Addr, SrcTy, "storetmp");
       }
     }
   }
 
   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);
     return;
   }
 
   llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile);
   if (isNontemporal) {
     llvm::MDNode *Node =
         llvm::MDNode::get(Store->getContext(),
                           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);
-  }
-}
+
 
 void CodeGenFunction::EmitStoreOfScalar(llvm::Value *value, LValue lvalue,
                                         bool isInit) {
   EmitStoreOfScalar(value, lvalue.getAddress(), lvalue.isVolatile(),
                     lvalue.getType(), lvalue.getBaseInfo(),
                     lvalue.getTBAAInfo(), isInit, lvalue.isNontemporal());
 }
 
addr = Builder.CreateElementBitCast(addr,
                                       CGM.getTypes().ConvertTypeForMem(type),
                                       field->getName());
 
   if (field->hasAttr<AnnotateAttr>())
     addr = EmitFieldAnnotations(field, addr);
 
   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;
 }
 
 LValue
 CodeGenFunction::EmitLValueForFieldInitialization(LValue Base,
                                                   const FieldDecl *Field) {
   QualType FieldType = Field->getType();
 

cfe/trunk/lib/CodeGen/CGValue.h

class LValue {
   // this lvalue.
   bool Nontemporal : 1;
 
   Expr *BaseIvarExp;
 
 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;
     this->Quals = Quals;
     this->Alignment = Alignment.getQuantity();
     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;
     this->ImpreciseLifetime = false;
     this->Nontemporal = false;
     this->ThreadLocalRef = false;
     this->BaseIvarExp = nullptr;
   }
public:
   }
   
   Expr *getBaseIvarExp() const { return BaseIvarExp; }
   void setBaseIvarExp(Expr *V) { BaseIvarExp = V; }
 
   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; }
 
   unsigned getAddressSpace() const { return Quals.getAddressSpace(); }
 
   CharUnits getAlignment() const { return CharUnits::fromQuantity(Alignment); }
   void setAlignment(CharUnits A) { Alignment = A.getQuantity(); }
 
public:
   const CGBitFieldInfo &getBitFieldInfo() const {
     assert(isBitField());
     return *BitFieldInfo;
   }
 
   // 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;
   }
 
   static LValue MakeVectorElt(Address vecAddress, llvm::Value *Idx,
                               QualType type, LValueBaseInfo BaseInfo) {
     LValue R;
     R.LVType = VectorElt;
     R.V = vecAddress.getPointer();

cfe/trunk/lib/CodeGen/CodeGenFunction.h

#endif
   //===--------------------------------------------------------------------===//
   //                                  Helpers
   //===--------------------------------------------------------------------===//
 
   LValue MakeAddrLValue(Address Addr, QualType T,
                         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);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
   CharUnits getNaturalTypeAlignment(QualType T,
                                     LValueBaseInfo *BaseInfo = nullptr,
                                     bool forPointeeType = false);
   CharUnits getNaturalPointeeTypeAlignment(QualType T,
public:
 
   /// EmitLoadOfScalar - Load a scalar value from an address, taking
   /// care to appropriately convert from the memory representation to
   /// the LLVM value representation.
   llvm::Value *EmitLoadOfScalar(Address Addr, bool Volatile, QualType Ty,
                                 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
   /// care to appropriately convert from the memory representation to
   /// the LLVM value representation.  The l-value must be a simple
   /// l-value.
   llvm::Value *EmitLoadOfScalar(LValue lvalue, SourceLocation Loc);
 
   /// EmitStoreOfScalar - Store a scalar value to an address, taking
   /// care to appropriately convert from the memory representation to
   /// the LLVM value representation.
   void EmitStoreOfScalar(llvm::Value *Value, Address Addr,
                          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
   /// care to appropriately convert from the memory representation to
   /// the LLVM value representation.  The l-value must be a simple
   /// l-value.  The isInit flag indicates whether this is an initialization.
   /// If so, atomic qualifiers are ignored and the store is always non-atomic.
   void EmitStoreOfScalar(llvm::Value *value, LValue lvalue, bool isInit=false);

cfe/trunk/lib/CodeGen/CodeGenFunction.cpp

CharUnits CodeGenFunction::getNaturalTypeAlignment(QualType T,
   }
   return Alignment;
 }
 
 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,
 /// construct an l-value with the natural pointee alignment of T.
 LValue
 CodeGenFunction::MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T) {
   LValueBaseInfo BaseInfo;
   CharUnits Align = getNaturalTypeAlignment(T, &BaseInfo, /*pointee*/ true);

cfe/trunk/lib/CodeGen/CodeGenModule.h

public:
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo();
 
   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);
 
   /// getTBAAMayAliasAccessInfo - Get TBAA information that represents
   /// may-alias accesses.
   TBAAAccessInfo getTBAAMayAliasAccessInfo();
 
   bool isTypeConstant(QualType QTy, bool ExcludeCtorDtor);

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

 }
 
 llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType QTy) {
   if (!TBAA)
     return nullptr;
   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;
   return TBAA->getAccessTagInfo(Info);
 }
 
 TBAAAccessInfo CodeGenModule::getTBAAMayAliasAccessInfo() {
   if (!TBAA)

cfe/trunk/lib/CodeGen/CodeGenTBAA.h

namespace clang {
   class LangOptions;
   class MangleContext;
   class QualType;
   class Type;
 
 namespace CodeGen {
 class CGRecordLayout;
 
-struct TBAAPathTag {
-  TBAAPathTag(const Type *B, const llvm::MDNode *A, uint64_t O)
-    : BaseT(B), AccessN(A), Offset(O) {}
-  const Type *BaseT;
-  const llvm::MDNode *AccessN;
-  uint64_t Offset;
-};
-
 // TBAAAccessInfo - Describes a memory access in terms of TBAA.
 struct TBAAAccessInfo {
-  TBAAAccessInfo(QualType BaseType, llvm::MDNode *AccessType, uint64_t Offset)
+  TBAAAccessInfo(llvm::MDNode *BaseType, llvm::MDNode *AccessType,
+                 uint64_t Offset)
     : BaseType(BaseType), AccessType(AccessType), Offset(Offset)
   {}
 
   explicit TBAAAccessInfo(llvm::MDNode *AccessType)
-    : TBAAAccessInfo(/* BaseType= */ QualType(), AccessType, /* Offset= */ 0)
+    : TBAAAccessInfo(/* BaseType= */ nullptr, AccessType, /* Offset= */ 0)
   {}
 
   TBAAAccessInfo()
     : TBAAAccessInfo(/* AccessType= */ nullptr)
   {}
 
   /// BaseType - The base/leading access type. May be null if this access
   /// descriptor represents an access that is not considered to be an access
   /// to an aggregate or union member.
-  QualType BaseType;
+  llvm::MDNode *BaseType;
 
   /// AccessType - The final access type. May be null if there is no TBAA
   /// information available about this access.
   llvm::MDNode *AccessType;
 
   /// Offset - The byte offset of the final access within the base one. Must be
   /// zero if the base access type is not specified.
   uint64_t Offset;
 };
 
 /// CodeGenTBAA - This class organizes the cross-module state that is used
 /// while lowering AST types to LLVM types.
 class CodeGenTBAA {
   ASTContext &Context;
   const CodeGenOptions &CodeGenOpts;
   const LangOptions &Features;
   MangleContext &MContext;
 
   // MDHelper - Helper for creating metadata.
   llvm::MDBuilder MDHelper;
 
   /// MetadataCache - This maps clang::Types to scalar llvm::MDNodes describing
   /// them.
   llvm::DenseMap<const Type *, llvm::MDNode *> MetadataCache;
-  /// This maps clang::Types to a struct node in the type DAG.
-  llvm::DenseMap<const Type *, llvm::MDNode *> StructTypeMetadataCache;
-  /// This maps TBAAPathTags to a tag node.
-  llvm::DenseMap<TBAAPathTag, llvm::MDNode *> StructTagMetadataCache;
+  /// This maps clang::Types to a base access type in the type DAG.
+  llvm::DenseMap<const Type *, llvm::MDNode *> BaseTypeMetadataCache;
+  /// This maps TBAA access descriptors to tag nodes.
+  llvm::DenseMap<TBAAAccessInfo, llvm::MDNode *> AccessTagMetadataCache;
 
   /// StructMetadataCache - This maps clang::Types to llvm::MDNodes describing
   /// them for struct assignments.
   llvm::DenseMap<const Type *, llvm::MDNode *> StructMetadataCache;
 
   llvm::MDNode *Root;
   llvm::MDNode *Char;
 
public:
   /// getVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getVTablePtrAccessInfo();
 
   /// getTBAAStructInfo - Get the TBAAStruct MDNode to be used for a memcpy of
   /// the given type.
   llvm::MDNode *getTBAAStructInfo(QualType QTy);
 
-  /// getBaseTypeInfo - Get metadata node for a given base access type.
-  llvm::MDNode *getBaseTypeInfo(QualType QType);
+  /// getBaseTypeInfo - 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 *getBaseTypeInfo(QualType QTy);
 
   /// getAccessTagInfo - Get TBAA tag for a given memory access.
   llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
 
   /// getMayAliasAccessInfo - Get TBAA information that represents may-alias
   /// accesses.
   TBAAAccessInfo getMayAliasAccessInfo();
 };
 
 }  // end namespace CodeGen
 }  // end namespace clang
 
 namespace llvm {
 
-template<> struct DenseMapInfo<clang::CodeGen::TBAAPathTag> {
-  static clang::CodeGen::TBAAPathTag getEmptyKey() {
-    return clang::CodeGen::TBAAPathTag(
-      DenseMapInfo<const clang::Type *>::getEmptyKey(),
-      DenseMapInfo<const MDNode *>::getEmptyKey(),
+template<> struct DenseMapInfo<clang::CodeGen::TBAAAccessInfo> {
+  static clang::CodeGen::TBAAAccessInfo getEmptyKey() {
+    return clang::CodeGen::TBAAAccessInfo(
+      DenseMapInfo<MDNode *>::getEmptyKey(),
+      DenseMapInfo<MDNode *>::getEmptyKey(),
       DenseMapInfo<uint64_t>::getEmptyKey());
   }
 
-  static clang::CodeGen::TBAAPathTag getTombstoneKey() {
-    return clang::CodeGen::TBAAPathTag(
-      DenseMapInfo<const clang::Type *>::getTombstoneKey(),
-      DenseMapInfo<const MDNode *>::getTombstoneKey(),
+  static clang::CodeGen::TBAAAccessInfo getTombstoneKey() {
+    return clang::CodeGen::TBAAAccessInfo(
+      DenseMapInfo<MDNode *>::getTombstoneKey(),
+      DenseMapInfo<MDNode *>::getTombstoneKey(),
       DenseMapInfo<uint64_t>::getTombstoneKey());
   }
 
-  static unsigned getHashValue(const clang::CodeGen::TBAAPathTag &Val) {
-    return DenseMapInfo<const clang::Type *>::getHashValue(Val.BaseT) ^
-           DenseMapInfo<const MDNode *>::getHashValue(Val.AccessN) ^
+  static unsigned getHashValue(const clang::CodeGen::TBAAAccessInfo &Val) {
+    return DenseMapInfo<MDNode *>::getHashValue(Val.BaseType) ^
+           DenseMapInfo<MDNode *>::getHashValue(Val.AccessType) ^
            DenseMapInfo<uint64_t>::getHashValue(Val.Offset);
   }
 
-  static bool isEqual(const clang::CodeGen::TBAAPathTag &LHS,
-                      const clang::CodeGen::TBAAPathTag &RHS) {
-    return LHS.BaseT == RHS.BaseT &&
-           LHS.AccessN == RHS.AccessN &&
+  static bool isEqual(const clang::CodeGen::TBAAAccessInfo &LHS,
+                      const clang::CodeGen::TBAAAccessInfo &RHS) {
+    return LHS.BaseType == RHS.BaseType &&
+           LHS.AccessType == RHS.AccessType &&
            LHS.Offset == RHS.Offset;
   }
 };
 
 }  // end namespace llvm
 
 #endif

cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp

if (CollectFields(0, QTy, Fields, TypeHasMayAlias(QTy)))
     return MDHelper.createTBAAStructNode(Fields);
 
   // For now, handle any other kind of type conservatively.
   return StructMetadataCache[Ty] = nullptr;
 }
 
 /// 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())
       return false;
     // RD can be struct, union, class, interface or enum.
     // For now, we only handle struct and class.
     if (RD->isStruct() || RD->isClass())
       return true;
   }
   return false;
 }
 
 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>()) {
     const RecordDecl *RD = TTy->getDecl()->getDefinition();
 
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
     SmallVector <std::pair<llvm::MDNode*, uint64_t>, 4> Fields;
     unsigned idx = 0;
     for (RecordDecl::field_iterator i = RD->field_begin(),
          e = RD->field_end(); i != e; ++i, ++idx) {
       QualType FieldQTy = i->getType();
       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()));
     }
 
     SmallString<256> OutName;
     if (Features.CPlusPlus) {
       // Don't use the mangler for C code.
       llvm::raw_svector_ostream Out(OutName);
       MContext.mangleTypeName(QualType(Ty, 0), Out);
     } else {
       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) {
   if (!Info.AccessType)
     return nullptr;
 
   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() {
   return TBAAAccessInfo(getChar());
 }

cfe/trunk/test/CodeGen/tbaa-for-vptr.cpp

 }
 
 void CallFoo(A *a, int (A::*fp)() const) {
   a->foo();
   (a->*fp)();
 }
 
 // CHECK-LABEL: @_Z7CallFoo
-// CHECK: %{{.*}} = load {{.*}} !tbaa ![[NUM:[0-9]+]]
+// CHECK: %{{.*}} = load i32 (%struct.A*)**, {{.*}} !tbaa ![[NUM:[0-9]+]]
 // CHECK: br i1
-// CHECK: load {{.*}}, !tbaa ![[NUM]]
+// CHECK: load i8*, {{.*}}, !tbaa ![[NUM]]
 //
 // CHECK-LABEL: @_ZN1AC2Ev
-// CHECK: store {{.*}} !tbaa ![[NUM]]
+// CHECK: store i32 (...)** {{.*}}, !tbaa ![[NUM]]
 //
 // CHECK: [[NUM]] = !{[[TYPE:!.*]], [[TYPE]], i64 0}
 // CHECK: [[TYPE]] = !{!"vtable pointer", !{{.*}}
 // NOTBAA-NOT: = !{!"Simple C++ TBAA"}