[CodeGen] Fix generation of TBAA tags for may-alias accesses

[kosarev]

Summary

This patch fixes creating TBAA access descriptors for may_alias-marked access types. Currently, for such types we generate ordinary descriptors with char as its access type. The patch changes this to produce proper may-alias descriptors.

[rjmccall]

lib/CodeGen/CodeGenTBAA.h

67

Hmm. I think you should talk this part over with Hal. If the size should be ignored then I think it's better to just leave this as 0; it's certainly easier to recognize 0 as a "size unknown" value.

[kosarev]

lib/CodeGen/CodeGenTBAA.h

67

Hal, in D41501 we use UINT64_MAX as the "unknown" size value. Should we do the same here?

[kosarev]

Hal, will you please take a look?

[kosarev]

Hal, a friendly ping.

[kosarev]

John, the patch hangs for about a month now and Hal still seems to be extremely busy. Can we commit this with the constant to be whatever you think is most correct and the fix it as needed and if needed later? At this very moment we don't analyze the size fields of tags anyway, so the point of this patch is mostly to handle may-alias-marked types correctly.

[kosarev]

Do not change the size field of may-alias TBAA descriptors for now.

[rjmccall]

I'm fine with that plan. LGTM.

[hfinkel]

In D42366#1012211, @rjmccall wrote:

I'm fine with that plan. LGTM.

Thanks.

We should probably discuss what we want to do with unknown-sized things. UINT64_MAX is the conservative answer, but maybe there's some value in just having 0 mean "unknown size" so we don't need to encode a bunch of UINT64_MAX values at the IR level.

[kosarev]

I think zero would serve better as the unknown-size value. People who are not aware of TBAA internals would guess that since zero-sized accesses make no sense, they are likely to have some special meaning. Similarly, for code that is supposed to process the size fields of access descriptors zero would be an obvious "illegal size value". In contrast, UINT64_MAX is just a very large number that doesn't hint anything on its special purpose.

Either way, we should reflect the convention in the documentation, D40975.

[rjmccall]

In D42366#1014157, @kosarev wrote:

I think zero would serve better as the unknown-size value. People who are not aware of TBAA internals would guess that since zero-sized accesses make no sense, they are likely to have some special meaning. Similarly, for code that is supposed to process the size fields of access descriptors zero would be an obvious "illegal size value". In contrast, UINT64_MAX is just a very large number that doesn't hint anything on its special purpose.

My thoughts exactly.

John.

[hfinkel]

In D42366#1014546, @rjmccall wrote:

In D42366#1014157, @kosarev wrote:

I think zero would serve better as the unknown-size value. People who are not aware of TBAA internals would guess that since zero-sized accesses make no sense, they are likely to have some special meaning. Similarly, for code that is supposed to process the size fields of access descriptors zero would be an obvious "illegal size value". In contrast, UINT64_MAX is just a very large number that doesn't hint anything on its special purpose.

My thoughts exactly.

John.

SGTM. Let's do that.

lib/CodeGen/CodeGenModule.cpp

 
 llvm::MDNode *CodeGenModule::getTBAATypeInfo(QualType QTy) {
   if (!TBAA)
     return nullptr;
   return TBAA->getTypeInfo(QTy);
 }
 
 TBAAAccessInfo CodeGenModule::getTBAAAccessInfo(QualType AccessType) {
-  // Pointee values may have incomplete types, but they shall never be
-  // dereferenced.
-  if (AccessType->isIncompleteType())
-    return TBAAAccessInfo::getIncompleteInfo();
-
-  uint64_t Size = Context.getTypeSizeInChars(AccessType).getQuantity();
-  return TBAAAccessInfo(getTBAATypeInfo(AccessType), Size);
+  if (!TBAA)
+    return TBAAAccessInfo();
+  return TBAA->getAccessInfo(AccessType);
 }
 
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
     return TBAAAccessInfo();
   return TBAA->getVTablePtrAccessInfo(VTablePtrType);
 }

lib/CodeGen/CodeGenTBAA.h

struct TBAAAccessInfo {
 
   TBAAAccessInfo()
     : TBAAAccessInfo(/* AccessType= */ nullptr, /* Size= */ 0)
   {}
 
   static TBAAAccessInfo getMayAliasInfo() {
     return TBAAAccessInfo(TBAAAccessKind::MayAlias,
                           /* BaseType= */ nullptr, /* AccessType= */ nullptr,
                           /* Offset= */ 0, /* Size= */ 0);
   }
 
   bool isMayAlias() const { return Kind == TBAAAccessKind::MayAlias; }
 
   static TBAAAccessInfo getIncompleteInfo() {
     return TBAAAccessInfo(TBAAAccessKind::Incomplete,
                           /* BaseType= */ nullptr, /* AccessType= */ nullptr,
                           /* Offset= */ 0, /* Size= */ 0);
public:
   CodeGenTBAA(ASTContext &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
               const LangOptions &Features, MangleContext &MContext);
   ~CodeGenTBAA();
 
   /// getTypeInfo - Get metadata used to describe accesses to objects of the
   /// given type.
   llvm::MDNode *getTypeInfo(QualType QTy);
 
+  /// getAccessInfo - Get TBAA information that describes an access to
+  /// an object of the given type.
+  TBAAAccessInfo getAccessInfo(QualType AccessType);
+
   /// getVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getVTablePtrAccessInfo(llvm::Type *VTablePtrType);
 
   /// getTBAAStructInfo - Get the TBAAStruct MDNode to be used for a memcpy of
   /// the given type.
   llvm::MDNode *getTBAAStructInfo(QualType QTy);
 

lib/CodeGen/CodeGenTBAA.cpp

llvm::MDNode *CodeGenTBAA::getTypeInfo(QualType QTy) {
 
   // Note that the following helper call is allowed to add new nodes to the
   // cache, which invalidates all its previously obtained iterators. So we
   // first generate the node for the type and then add that node to the cache.
   llvm::MDNode *TypeNode = getTypeInfoHelper(Ty);
   return MetadataCache[Ty] = TypeNode;
 }
 
+TBAAAccessInfo CodeGenTBAA::getAccessInfo(QualType AccessType) {
+  // Pointee values may have incomplete types, but they shall never be
+  // dereferenced.
+  if (AccessType->isIncompleteType())
+    return TBAAAccessInfo::getIncompleteInfo();
+
+  if (TypeHasMayAlias(AccessType))
+    return TBAAAccessInfo::getMayAliasInfo();
+
+  uint64_t Size = Context.getTypeSizeInChars(AccessType).getQuantity();
+  return TBAAAccessInfo(getTypeInfo(AccessType), Size);
+}
+
 TBAAAccessInfo CodeGenTBAA::getVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   llvm::DataLayout DL(&Module);
   unsigned Size = DL.getPointerTypeSize(VTablePtrType);
   return TBAAAccessInfo(createScalarTypeNode("vtable pointer", getRoot(), Size),
                         Size);
 }
 
 bool

test/CodeGen/may-alias.c

-// RUN: %clang_cc1 -Werror -triple i386-unknown-unknown -emit-llvm -O1 -no-struct-path-tbaa -disable-llvm-passes -o - %s | FileCheck %s
-// RUN: %clang_cc1 -Werror -triple i386-unknown-unknown -emit-llvm -O1 -disable-llvm-passes -o - %s | FileCheck %s -check-prefix=PATH
+// RUN: %clang_cc1 -Werror -triple i386-unknown-unknown -emit-llvm -O1 \
+// RUN:     -no-struct-path-tbaa -disable-llvm-passes -o - %s | \
+// RUN:     FileCheck %s -check-prefixes=CHECK,SCALAR
+// RUN: %clang_cc1 -Werror -triple i386-unknown-unknown -emit-llvm -O1 \
+// RUN:     -disable-llvm-passes -o - %s | \
+// RUN:     FileCheck %s -check-prefixes=CHECK,OLD-PATH
+// RUN: %clang_cc1 -Werror -triple i386-unknown-unknown -emit-llvm -O1 \
+// RUN:     -new-struct-path-tbaa -disable-llvm-passes -o - %s | \
+// RUN:     FileCheck %s -check-prefixes=CHECK,NEW-PATH
 
 // Types with the may_alias attribute should be considered equivalent
 // to char for aliasing.
 
 typedef int __attribute__((may_alias)) aliasing_int;
 
-void test0(aliasing_int *ai, int *i)
-{
-// CHECK: store i32 0, i32* %{{.*}}, !tbaa [[TAG_CHAR:!.*]]
-// PATH: store i32 0, i32* %{{.*}}, !tbaa [[TAG_CHAR:!.*]]
+void test0(aliasing_int *ai, int *i) {
+// CHECK-LABEL: test0
+// CHECK: store i32 0, {{.*}}, !tbaa [[TAG_alias_int:!.*]]
   *ai = 0;
-// CHECK: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
-// PATH: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+
+// CHECK: store i32 1, {{.*}}, !tbaa [[TAG_int:!.*]]
   *i = 1;
 }
 
 // PR9307
 struct Test1 { int x; };
 struct Test1MA { int x; } __attribute__((may_alias));
 void test1(struct Test1MA *p1, struct Test1 *p2) {
-  // CHECK: store i32 2, i32* {{%.*}}, !tbaa [[TAG_CHAR]]
-  // PATH: store i32 2, i32* {{%.*}}, !tbaa [[TAG_CHAR]]
+// CHECK-LABEL: test1
+// CHECK: store i32 2, {{.*}}, !tbaa [[TAG_alias_test1_x:!.*]]
   p1->x = 2;
-  // CHECK: store i32 3, i32* {{%.*}}, !tbaa [[TAG_INT]]
-  // PATH: store i32 3, i32* {{%.*}}, !tbaa [[TAG_test1_x:!.*]]
+
+// CHECK: store i32 3, {{.*}}, !tbaa [[TAG_test1_x:!.*]]
   p2->x = 3;
 }
-// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
-// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
-// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C/C++ TBAA"}
-// CHECK: [[TAG_CHAR]] = !{[[TYPE_CHAR]], [[TYPE_CHAR]], i64 0}
-// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
-// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
-
-// PATH: [[TYPE_CHAR:!.*]] = !{!"omnipotent char", !{{.*}}
-// PATH: [[TAG_CHAR]] = !{[[TYPE_CHAR]], [[TYPE_CHAR]], i64 0}
-// PATH: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
-// PATH: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
-// PATH: [[TAG_test1_x]] = !{[[TYPE_test1:!.*]], [[TYPE_INT]], i64 0}
-// PATH: [[TYPE_test1]] = !{!"Test1", [[TYPE_INT]], i64 0}
+
+// SCALAR-DAG: [[ROOT:!.*]] = !{!"Simple C/C++ TBAA"}
+// SCALAR-DAG: [[TYPE_char:!.*]] = !{!"omnipotent char", [[ROOT]], i64 0}
+// SCALAR-DAG: [[TAG_alias_int]] = !{[[TYPE_char]], [[TYPE_char]], i64 0}
+// SCALAR-DAG: [[TAG_alias_test1_x]] = !{[[TYPE_char]], [[TYPE_char]], i64 0}
+// SCALAR-DAG: [[TYPE_int:!.*]] = !{!"int", [[TYPE_char]], i64 0}
+// SCALAR-DAG: [[TAG_int]] = !{[[TYPE_int]], [[TYPE_int]], i64 0}
+// SCALAR-DAG: [[TAG_test1_x]] = !{[[TYPE_int]], [[TYPE_int]], i64 0}
+
+// OLD-PATH-DAG: [[ROOT:!.*]] = !{!"Simple C/C++ TBAA"}
+// OLD-PATH-DAG: [[TYPE_char:!.*]] = !{!"omnipotent char", [[ROOT]], i64 0}
+// OLD-PATH-DAG: [[TAG_alias_int]] = !{[[TYPE_char]], [[TYPE_char]], i64 0}
+// OLD-PATH-DAG: [[TAG_alias_test1_x]] = !{[[TYPE_char]], [[TYPE_char]], i64 0}
+// OLD-PATH-DAG: [[TYPE_int:!.*]] = !{!"int", [[TYPE_char]], i64 0}
+// OLD-PATH-DAG: [[TAG_int]] = !{[[TYPE_int]], [[TYPE_int]], i64 0}
+// OLD-PATH-DAG: [[TYPE_test1:!.*]] = !{!"Test1", [[TYPE_int]], i64 0}
+// OLD-PATH-DAG: [[TAG_test1_x]] = !{[[TYPE_test1]], [[TYPE_int]], i64 0}
+
+// NEW-PATH-DAG: [[ROOT:!.*]] = !{!"Simple C/C++ TBAA"}
+// NEW-PATH-DAG: [[TYPE_char:!.*]] = !{[[ROOT]], i64 1, !"omnipotent char"}
+// NEW-PATH-DAG: [[TAG_alias_int]] = !{[[TYPE_char]], [[TYPE_char]], i64 0, i64 0}
+// NEW-PATH-DAG: [[TAG_alias_test1_x]] = !{[[TYPE_char]], [[TYPE_char]], i64 0, i64 0}
+// NEW-PATH-DAG: [[TYPE_int:!.*]] = !{[[TYPE_char]], i64 4, !"int"}
+// NEW-PATH-DAG: [[TAG_int]] = !{[[TYPE_int]], [[TYPE_int]], i64 0, i64 4}
+// NEW-PATH-DAG: [[TYPE_test1:!.*]] = !{[[TYPE_char]], i64 4, !"Test1", [[TYPE_int]], i64 0, i64 4}
+// NEW-PATH-DAG: [[TAG_test1_x]] = !{[[TYPE_test1]], [[TYPE_int]], i64 0, i64 4}