[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@rsmith, thanks. Le'ts see if there is a need for the command-line option down 
the road.


Repository:
  rL LLVM

https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

I wonder if we should have a `-fno-strict-something` flag for this. I suppose 
there's no more effective way to find out than to try this and see what happens.




Comment at: lib/CodeGen/CGCall.cpp:2052
+//
+// We intentionally threat this behaviour as undefined by marking 
'this'
+// with noalias attribute and have recommended the Standard to be

thread -> treat


https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@rsmith do you have any comments or suggestions?


https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.

LGTM.




Comment at: lib/CodeGen/CGCall.cpp:2049
+// from the constructor’s this pointer, the value of the object or
+// subobject thus obtained is unspecified.
+unsigned ThisIRArg, NumIRArgs;

rjmccall wrote:
> You probably ought to add here that we're treating this as undefined behavior 
> and have recommended the standard be changed.
Thanks, looks good.


https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1893
+
+IsCtor = isa(TargetDecl);
   }

AntonBikineev wrote:
> rjmccall wrote:
> > I feel like you should just use `TargetDecl && 
> > isa(TargetDecl)` below; it's more obvious.
> > 
> > Is there not an analogous rule for destructors?
> There appears to be no similar rule for destructors, maybe because at the 
> point of calling a destructor the value of the object/subobjects is 
> well-determined.
That's a good point, although there's certainly a point during the destructor's 
execution that that's no longer true.



Comment at: lib/CodeGen/CGCall.cpp:2049
+// from the constructor’s this pointer, the value of the object or
+// subobject thus obtained is unspecified.
+unsigned ThisIRArg, NumIRArgs;

You probably ought to add here that we're treating this as undefined behavior 
and have recommended the standard be changed.


https://reviews.llvm.org/D46441



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