[Metadata] Add TBAA struct metadata to `AAMDNode`

[anton-afanasyev]

Summary

Make AAMDNodes' getAAMetadata() and setAAMetadata() to take !tbaa.struct
into account like for !tbaa. This impacts llvm.org/pr42022.
This is a temprorary fix needed to keep !tbaa.struct tag by SROA pass. New field TBAAStruct should be deleted when !tbaa tag replaces !tbaa.struct. Merging two !tbaa.struct's to one is conservatively considered to be nullptr (giving MayAlias) -- this could be enhanced, but relying on the said replacement.

[hfinkel]

llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp

527

Does MDNode::getMostGenericTBAA actually support struct.tbaa metadata? I didn't think that it did.

[anton-afanasyev]

llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp

527

No, it really didn't. Fixing it.

[anton-afanasyev]

Hi @hfinkel, I've investigated the current state of TBAA support in LLVM. AFAIU (see D40975, but it is still uncommited) the new format of tbaa tag replaces tbaa.struct tag. Does it mean tbaa.struct tag will be deprecated soon?

[anton-afanasyev]

Updated getAAMetadata() with Merge=true

[anton-afanasyev]

llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp

527

Updated. I conservatively suppose that merging two !tbaa.struct's turns to MayAlias.

[anton-afanasyev]

Fixed

[anton-afanasyev]

In D70924#1780145, @anton-afanasyev wrote:

Hi @hfinkel, I've investigated the current state of TBAA support in LLVM. AFAIU (see D40975, but it is still uncommited) the new format of tbaa tag replaces tbaa.struct tag. Does it mean tbaa.struct tag will be deprecated soon?

This replacing !tbaa.struct->!tbaa is described at the topic http://lists.llvm.org/pipermail/llvm-dev/2017-October/118284.html started by @kosarev, and I see it has already been supported and almost commited (but not announced). So my patch looks like temprorary bug fixing until this transfer is finished.

[anton-afanasyev]

Fix

[hfinkel]

In D70924#1780145, @anton-afanasyev wrote:

Hi @hfinkel, I've investigated the current state of TBAA support in LLVM. AFAIU (see D40975, but it is still uncommited) the new format of tbaa tag replaces tbaa.struct tag. Does it mean tbaa.struct tag will be deprecated soon?

It might, but it depends on the progress on the new TBAA format. This is not a large change, and I think that we can proceed with this regardless.

Can you please explain what will end up using this information? That should be present in the description.

[anton-afanasyev]

In D70924#1782238, @hfinkel wrote:

In D70924#1780145, @anton-afanasyev wrote:

Hi @hfinkel, I've investigated the current state of TBAA support in LLVM. AFAIU (see D40975, but it is still uncommited) the new format of tbaa tag replaces tbaa.struct tag. Does it mean tbaa.struct tag will be deprecated soon?

It might, but it depends on the progress on the new TBAA format. This is not a large change, and I think that we can proceed with this regardless.

Can you please explain what will end up using this information? That should be present in the description.

Ok, updated summary.

[anton-afanasyev]

Gently ping. Is it ok now?

[anton-afanasyev]

Gently ping again.

[anton-afanasyev]

Ping!

[RKSimon]

llvm/include/llvm/IR/Metadata.h

647

I always get worried when constructors with default args get rearranged like this. Maybe change to this:

explicit AAMDNodes() = default;
explicit AAMDNodes(MDNode *T, MDNode *TS, MDNode *S, MDNode *N)
    : TBAA(T), TBAAStruct(TS), Scope(S), NoAlias(N) {}

MDNode *TBAA = nullptr; // etc.

[anton-afanasyev]

Update AAMDNodes constructors

[anton-afanasyev]

llvm/include/llvm/IR/Metadata.h

647

Ok, done.

[RKSimon]

@hfinkel Any more comments?

[hfinkel]

In D70924#1804715, @RKSimon wrote:

@hfinkel Any more comments?

Nope. LGTM.

llvm/include/llvm/IR/Metadata.h

public:
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == MDStringKind;
   }
 };
 
 /// A collection of metadata nodes that might be associated with a
 /// memory access used by the alias-analysis infrastructure.
 struct AAMDNodes {
-  explicit AAMDNodes(MDNode *T = nullptr, MDNode *S = nullptr,
-                     MDNode *N = nullptr)
-      : TBAA(T), Scope(S), NoAlias(N) {}
+  explicit AAMDNodes() = default;
+  explicit AAMDNodes(MDNode *T, MDNode *TS, MDNode *S, MDNode *N)
+      : TBAA(T), TBAAStruct(TS), Scope(S), NoAlias(N) {}
 
   bool operator==(const AAMDNodes &A) const {
-    return TBAA == A.TBAA && Scope == A.Scope && NoAlias == A.NoAlias;
+    return TBAA == A.TBAA && TBAAStruct == A.TBAAStruct && Scope == A.Scope &&
+           NoAlias == A.NoAlias;
   }
 
   bool operator!=(const AAMDNodes &A) const { return !(*this == A); }
 
-  explicit operator bool() const { return TBAA || Scope || NoAlias; }
+  explicit operator bool() const {
+    return TBAA || TBAAStruct || Scope || NoAlias;
+  }
 
   /// The tag for type-based alias analysis.
-  MDNode *TBAA;
+  MDNode *TBAA = nullptr;
+
+  /// The tag for type-based alias analysis (tbaa struct).
+  MDNode *TBAAStruct = nullptr;
 
   /// The tag for alias scope specification (used with noalias).
-  MDNode *Scope;
+  MDNode *Scope = nullptr;
 
   /// The tag specifying the noalias scope.
-  MDNode *NoAlias;
+  MDNode *NoAlias = nullptr;
 
   /// Given two sets of AAMDNodes that apply to the same pointer,
   /// give the best AAMDNodes that are compatible with both (i.e. a set of
   /// nodes whose allowable aliasing conclusions are a subset of those
   /// allowable by both of the inputs). However, for efficiency
   /// reasons, do not create any new MDNodes.
   AAMDNodes intersect(const AAMDNodes &Other) {
     AAMDNodes Result;
     Result.TBAA = Other.TBAA == TBAA ? TBAA : nullptr;
+    Result.TBAAStruct = Other.TBAAStruct == TBAAStruct ? TBAAStruct : nullptr;
     Result.Scope = Other.Scope == Scope ? Scope : nullptr;
     Result.NoAlias = Other.NoAlias == NoAlias ? NoAlias : nullptr;
     return Result;
   }
 };
 
 // Specialize DenseMapInfo for AAMDNodes.
 template<>
 struct DenseMapInfo<AAMDNodes> {
   static inline AAMDNodes getEmptyKey() {
     return AAMDNodes(DenseMapInfo<MDNode *>::getEmptyKey(),
-                     nullptr, nullptr);
+                     nullptr, nullptr, nullptr);
   }
 
   static inline AAMDNodes getTombstoneKey() {
     return AAMDNodes(DenseMapInfo<MDNode *>::getTombstoneKey(),
-                     nullptr, nullptr);
+                     nullptr, nullptr, nullptr);
   }
 
   static unsigned getHashValue(const AAMDNodes &Val) {
     return DenseMapInfo<MDNode *>::getHashValue(Val.TBAA) ^
+           DenseMapInfo<MDNode *>::getHashValue(Val.TBAAStruct) ^
            DenseMapInfo<MDNode *>::getHashValue(Val.Scope) ^
            DenseMapInfo<MDNode *>::getHashValue(Val.NoAlias);
   }
 
   static bool isEqual(const AAMDNodes &LHS, const AAMDNodes &RHS) {
     return LHS == RHS;
   }
 };

llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp

while (IA >= 0 && IB >= 0) {
     --IA;
     --IB;
   }
 
   return Ret;
 }
 
 void Instruction::getAAMetadata(AAMDNodes &N, bool Merge) const {
-  if (Merge)
+  if (Merge) {
     N.TBAA =
         MDNode::getMostGenericTBAA(N.TBAA, getMetadata(LLVMContext::MD_tbaa));
-  else
-    N.TBAA = getMetadata(LLVMContext::MD_tbaa);
-
-  if (Merge)
+    N.TBAAStruct = nullptr;
     N.Scope = MDNode::getMostGenericAliasScope(
         N.Scope, getMetadata(LLVMContext::MD_alias_scope));
-  else
-    N.Scope = getMetadata(LLVMContext::MD_alias_scope);
-
-  if (Merge)
     N.NoAlias =
         MDNode::intersect(N.NoAlias, getMetadata(LLVMContext::MD_noalias));
-  else
+  } else {
+    N.TBAA = getMetadata(LLVMContext::MD_tbaa);
+    N.TBAAStruct = getMetadata(LLVMContext::MD_tbaa_struct);
+    N.Scope = getMetadata(LLVMContext::MD_alias_scope);
     N.NoAlias = getMetadata(LLVMContext::MD_noalias);
   }
+}
 
 static const MDNode *createAccessTag(const MDNode *AccessType) {
   // If there is no access type or the access type is the root node, then
   // we don't have any useful access tag to return.
   if (!AccessType || AccessType->getNumOperands() < 2)
     return nullptr;
 
   Type *Int64 = IntegerType::get(AccessType->getContext(), 64);

llvm/lib/IR/Metadata.cpp

if (!Info.empty())
     return;
 
   getContext().pImpl->InstructionMetadata.erase(this);
   setHasMetadataHashEntry(false);
 }
 
 void Instruction::setAAMetadata(const AAMDNodes &N) {
   setMetadata(LLVMContext::MD_tbaa, N.TBAA);
+  setMetadata(LLVMContext::MD_tbaa_struct, N.TBAAStruct);
   setMetadata(LLVMContext::MD_alias_scope, N.Scope);
   setMetadata(LLVMContext::MD_noalias, N.NoAlias);
 }
 
 MDNode *Instruction::getMetadataImpl(unsigned KindID) const {
   // Handle 'dbg' as a special case since it is not stored in the hash table.
   if (KindID == LLVMContext::MD_dbg)
     return DbgLoc.getAsMDNode();

llvm/test/Transforms/SROA/tbaa-struct.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -sroa %s | FileCheck %s
 
 ; SROA should keep `!tbaa.struct` metadata
 
 %vector = type { float, float }
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* writeonly, i8* readonly, i64, i1 immarg)
 declare <2 x float> @foo(%vector* %0)
 
 define void @bar(%vector* %y2) {
 ; CHECK-LABEL: @bar(
 ; CHECK-NEXT:    [[X14:%.*]] = call <2 x float> @foo(%vector* [[Y2:%.*]])
 ; CHECK-NEXT:    [[X7_SROA_0_0_X18_SROA_CAST:%.*]] = bitcast %vector* [[Y2]] to <2 x float>*
-; CHECK-NEXT:    store <2 x float> [[X14]], <2 x float>* [[X7_SROA_0_0_X18_SROA_CAST]], align 4
+; CHECK-NEXT:    store <2 x float> [[X14]], <2 x float>* [[X7_SROA_0_0_X18_SROA_CAST]], align 4, !tbaa.struct !0
 ; CHECK-NEXT:    ret void
 ;
   %x7 = alloca %vector
   %x14 = call <2 x float> @foo(%vector* %y2)
   %x15 = bitcast %vector* %x7 to <2 x float>*
   store <2 x float> %x14, <2 x float>* %x15
   %x19 = bitcast %vector* %x7 to i8*
   %x18 = bitcast %vector* %y2 to i8*