[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-26 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev abandoned this revision.
kosarev added a comment.

OK, thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/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.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-25 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added a comment.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-24 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

In https://reviews.llvm.org/D39138#905445, @rjmccall wrote:

> In https://reviews.llvm.org/D39138#905184, @hfinkel wrote:
>
> > In https://reviews.llvm.org/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 ;)


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D39138#905184, @hfinkel wrote:

> In https://reviews.llvm.org/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.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-24 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

In https://reviews.llvm.org/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.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-24 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added a comment.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

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?


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

2017-10-20 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev created this revision.
kosarev added a project: clang.

Repository:
  rL LLVM

https://reviews.llvm.org/D39138

Files:
  lib/CodeGen/CGCXXABI.cpp
  test/CodeGen/tbaa-this.cpp


Index: test/CodeGen/tbaa-this.cpp
===
--- test/CodeGen/tbaa-this.cpp
+++ test/CodeGen/tbaa-this.cpp
@@ -0,0 +1,21 @@
+// 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}
Index: lib/CodeGen/CGCXXABI.cpp
===
--- lib/CodeGen/CGCXXABI.cpp
+++ lib/CodeGen/CGCXXABI.cpp
@@ -152,9 +152,12 @@
 void CGCXXABI::EmitThisParam(CodeGenFunction ) {
   /// Initialize the 'this' slot.
   assert(getThisDecl(CGF) && "no 'this' variable for function");
-  CGF.CXXABIThisValue
-= CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(getThisDecl(CGF)),
- "this");
+  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 ,


Index: test/CodeGen/tbaa-this.cpp
===
--- test/CodeGen/tbaa-this.cpp
+++ test/CodeGen/tbaa-this.cpp
@@ -0,0 +1,21 @@
+// 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}
Index: lib/CodeGen/CGCXXABI.cpp
===
--- lib/CodeGen/CGCXXABI.cpp
+++ lib/CodeGen/CGCXXABI.cpp
@@ -152,9 +152,12 @@
 void CGCXXABI::EmitThisParam(CodeGenFunction ) {
   /// Initialize the 'this' slot.
   assert(getThisDecl(CGF) && "no 'this' variable for function");
-  CGF.CXXABIThisValue
-= CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(getThisDecl(CGF)),
- "this");
+  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 ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits