[CodeGen] Generate TBAA info for 'this' pointers

[rjmccall]

AFAIK, this is pointless because that alloca will be trivially eliminated by mem2reg. Am I missing something? Is this important for some sort of consistency check?

[kosarev]

Correct, they are eliminated. However, this change makes things a bit easier as we are pursuing undecorated instructions produced by clang. Once they have their TBAA tags, we don't need to guess if it's something trivial for the optimizer. It shouldn't be a problem to keep this as a local patch, though. So I'm fine with either way.

[rjmccall]

Okay, if this is just for your own checking, I'd rather not take it. It's not a significant compile-time cost, but there's no reason to pay it at all.

[hfinkel]

In D39138#904747, @rjmccall wrote:

Okay, if this is just for your own checking, I'd rather not take it. It's not a significant compile-time cost, but there's no reason to pay it at all.

In that case, can we take it? I'd rather have everything decorated for use by the type sanitizer.

[rjmccall]

In D39138#905184, @hfinkel wrote:

In D39138#904747, @rjmccall wrote:

Okay, if this is just for your own checking, I'd rather not take it. It's not a significant compile-time cost, but there's no reason to pay it at all.

In that case, can we take it? I'd rather have everything decorated for use by the type sanitizer.

There will never, ever be an illegal access to this alloca.

[hfinkel]

In D39138#905445, @rjmccall wrote:

In D39138#905184, @hfinkel wrote:

In D39138#904747, @rjmccall wrote:

Okay, if this is just for your own checking, I'd rather not take it. It's not a significant compile-time cost, but there's no reason to pay it at all.

In that case, can we take it? I'd rather have everything decorated for use by the type sanitizer.

There will never, ever be an illegal access to this alloca.

Never mind, then ;)

[kosarev]

Hmm, according to our research such loads constitute about 18% of all loads under -O -Xclang -disable-llvm-passes on the LLVM code base. I wonder, do you think it would be nice to not generate them at all? I mean, provided that necessary changes do not add too much special-case code.

[rjmccall]

In D39138#906623, @kosarev wrote:

Hmm, according to our research such loads constitute about 18% of all loads under -O -Xclang -disable-llvm-passes on the LLVM code base. I wonder, do you think it would be nice to not generate them at all? I mean, provided that necessary changes do not add too much special-case code.

It would be straightforward to not generate them, but it would create longer live ranges at -O0 and allow 'this' to disappear in debug info, both of which are undesirable.

[kosarev]

OK, thanks.

lib/CodeGen/CGCXXABI.cpp

void CGCXXABI::buildThisParam(CodeGenFunction &CGF, FunctionArgList &params) {
   } else {
     CGF.CXXABIThisAlignment = Layout.getNonVirtualAlignment();
   }
 }
 
 void CGCXXABI::EmitThisParam(CodeGenFunction &CGF) {
   /// Initialize the 'this' slot.
   assert(getThisDecl(CGF) && "no 'this' variable for function");
-  CGF.CXXABIThisValue
-    = CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(getThisDecl(CGF)),
+  ImplicitParamDecl *ThisPtrDecl = getThisDecl(CGF);
+  auto *Load = CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(ThisPtrDecl),
                                       "this");
+  QualType ThisPtrType = ThisPtrDecl->getType();
+  CGM.DecorateInstructionWithTBAA(Load, CGM.getTBAAAccessInfo(ThisPtrType));
+  CGF.CXXABIThisValue = Load;
 }
 
 void CGCXXABI::EmitReturnFromThunk(CodeGenFunction &CGF,
                                    RValue RV, QualType ResultType) {
   CGF.EmitReturnOfRValue(RV, ResultType);
 }
 
 CharUnits CGCXXABI::GetArrayCookieSize(const CXXNewExpr *expr) {

test/CodeGen/tbaa-this.cpp

+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
+// RUN:     -emit-llvm -o - | FileCheck %s
+//
+// Check that we generate correct TBAA information for 'this' pointers.
+
+struct C {
+  C *self();
+};
+
+C *C::self() {
+// CHECK-LABEL: _ZN1C4selfEv
+// CHECK: load {{.*}}, !tbaa [[TAG_pointer:!.*]]
+  return this;
+}
+
+C* foo(C *p) {
+  return p->self();
+}
+
+// CHECK-DAG: [[TAG_pointer]] = !{[[TYPE_pointer:!.*]], [[TYPE_pointer]], i64 0}
+// CHECK-DAG: [[TYPE_pointer]] = !{!"any pointer", !{{.*}}, i64 0}