[Analysis] Fix merging TBAA tags with different final access types

There are cases when we have to merge TBAA access tags with the
same base access type, but different final access types. For
example, accesses to different members of the same structure may
be vectorized into a single load or store instruction. Since we
currently assume that the tags to merge always share the same
final access type, we incorrectly return a tag that describes an
access to one of the original final access types as the generic
tag. This patch fixes that by producing generic tags for the
common type and not the final access types of the original tags.

Resolves:
PR35225: Wrong tbaa metadata after load store vectorizer due to
recent change
https://bugs.llvm.org/show_bug.cgi?id=35225

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@317682 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/TypeBasedAliasAnalysis.cpp b/lib/Analysis/TypeBasedAliasAnalysis.cpp
index 8812ca2..1e36e31 100644 (file)
--- a/lib/Analysis/TypeBasedAliasAnalysis.cpp
+++ b/lib/Analysis/TypeBasedAliasAnalysis.cpp
@@ -506,6 +506,11 @@ static bool findAccessType(TBAAStructTagNode BaseTag,
 }
 
 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);
   auto *ImmutabilityFlag = ConstantAsMetadata::get(ConstantInt::get(Int64, 0));
   Metadata *Ops[] = {const_cast<MDNode*>(AccessType),
@@ -537,42 +542,26 @@ static bool matchAccessTags(const MDNode *A, const MDNode *B,
   assert(isStructPathTBAA(B) && "Access B is not struct-path aware!");
 
   TBAAStructTagNode TagA(A), TagB(B);
+  const MDNode *CommonType = getLeastCommonType(TagA.getAccessType(),
+                                                TagB.getAccessType());
+  if (GenericTag)
+    *GenericTag = createAccessTag(CommonType);
 
   // TODO: We need to check if AccessType of TagA encloses AccessType of
   // TagB to support aggregate AccessType. If yes, return true.
 
-  const MDNode *BaseA = TagA.getBaseType();
-  const MDNode *BaseB = TagB.getBaseType();
-
   // Climb the type DAG from base type of A to see if we reach base type of B.
   uint64_t OffsetA;
-  if (findAccessType(TagA, BaseB, OffsetA)) {
-    if (GenericTag)
-      *GenericTag = createAccessTag(TagB.getAccessType());
+  if (findAccessType(TagA, TagB.getBaseType(), OffsetA))
     return OffsetA == TagB.getOffset();
-  }
 
   // Climb the type DAG from base type of B to see if we reach base type of A.
   uint64_t OffsetB;
-  if (findAccessType(TagB, BaseA, OffsetB)) {
-    if (GenericTag)
-      *GenericTag = createAccessTag(TagA.getAccessType());
+  if (findAccessType(TagB, TagA.getBaseType(), OffsetB))
     return OffsetB == TagA.getOffset();
-  }
-
-  // If neither node is an ancestor of the other, then try to find the type
-  // that is common to both the final access types.
-  const MDNode *CommonType = getLeastCommonType(TagA.getAccessType(),
-                                                TagB.getAccessType());
-
-  // If there is no common type or the only common type is the root node, then
-  // we don't have any useful generic access tag to return.
-  if (GenericTag)
-    *GenericTag = !CommonType || CommonType->getNumOperands() < 2 ?
-        nullptr : createAccessTag(CommonType);
 
-  // If they have different roots, they're part of different potentially
-  // unrelated type systems, so we must be conservative.
+  // If the final access types have different roots, they're part of different
+  // potentially unrelated type systems, so we must be conservative.
   if (!CommonType)
     return true;
 

diff --git a/test/Transforms/LoadStoreVectorizer/X86/merge-tbaa.ll b/test/Transforms/LoadStoreVectorizer/X86/merge-tbaa.ll
new file mode 100644 (file)
index 0000000..3c283dc
--- /dev/null
+++ b/test/Transforms/LoadStoreVectorizer/X86/merge-tbaa.ll
@@ -0,0 +1,46 @@
+; RUN: opt -mtriple=x86_64-unknown-linux-gnu -load-store-vectorizer -S < %s | \
+; RUN:     FileCheck %s
+;
+; The GPU Load & Store Vectorizer may merge differently-typed accesses into a
+; single instruction. This test checks that we merge TBAA tags for such
+; accesses correctly.
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; struct S {
+;   float f;
+;   int i;
+; };
+%struct.S = type { float, i32 }
+
+; float foo(S *p) {
+;   p->f -= 1;
+;   p->i -= 1;
+;   return p->f;
+; }
+define float @foo(%struct.S* %p) {
+entry:
+; CHECK-LABEL: foo
+; CHECK: load <2 x i32>, {{.*}}, !tbaa [[TAG_char:!.*]]
+; CHECK: store <2 x i32> {{.*}}, !tbaa [[TAG_char]]
+  %f = getelementptr inbounds %struct.S, %struct.S* %p, i64 0, i32 0
+  %0 = load float, float* %f, align 4, !tbaa !2
+  %sub = fadd float %0, -1.000000e+00
+  store float %sub, float* %f, align 4, !tbaa !2
+  %i = getelementptr inbounds %struct.S, %struct.S* %p, i64 0, i32 1
+  %1 = load i32, i32* %i, align 4, !tbaa !8
+  %sub1 = add nsw i32 %1, -1
+  store i32 %sub1, i32* %i, align 4, !tbaa !8
+  ret float %sub
+}
+
+!2 = !{!3, !4, i64 0}
+!3 = !{!"_ZTS1S", !4, i64 0, !7, i64 4}
+!4 = !{!"float", !5, i64 0}
+!5 = !{!"omnipotent char", !6, i64 0}
+!6 = !{!"Simple C++ TBAA"}
+!7 = !{!"int", !5, i64 0}
+!8 = !{!3, !7, i64 4}
+
+; CHECK-DAG: [[TYPE_char:!.*]] = !{!"omnipotent char", {{.*}}, i64 0}
+; CHECK-FAG: [[TAG_char]] = !{[[TYPE_char]], [[TYPE_char]], i64 0}