rjmccall closed this revision.
rjmccall added a comment.
Sure, r320721.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danzimm added a comment.
Also, I don't have commit access. Could someone else commit this for me?
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
danzimm added a comment.
@dexonsmith Here are my results after passing those extra flags with `-O3`
(/Users/danzimm/oss/build/bin/clang -cc1 -internal-isystem
/Users/danzimm/oss/build/lib/clang/6.0.0/include -nostdsysteminc -triple
x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O3
rjmccall added a comment.
Oh, yes, that could be.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dexonsmith added a comment.
In https://reviews.llvm.org/D41050#954863, @rjmccall wrote:
> Heh, alright, that works. It's unfortunate that -disable-llvm-passes doesn't
> suppress running the pass under -O0; that seems like an oversight.
>
> Anyway, LGTM.
I suspect `-O0` just generates differen
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Heh, alright, that works. It's unfortunate that -disable-llvm-passes doesn't
suppress running the pass under -O0; that seems like an oversight.
Anyway, LGTM.
Repository:
rC Clang
htt
danzimm updated this revision to Diff 126893.
danzimm added a comment.
Remove unnecessary checks from tests (sorry for unbatched updates)
Repository:
rC Clang
https://reviews.llvm.org/D41050
Files:
lib/CodeGen/CGClass.cpp
test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
Index: test/Code
danzimm updated this revision to Diff 126892.
danzimm added a comment.
Pass -disable-llvm-passes with -O3 so that we can test that the IR we generate
actually is generated
Repository:
rC Clang
https://reviews.llvm.org/D41050
Files:
lib/CodeGen/CGClass.cpp
test/CodeGenObjCXX/arc-forwarde
danzimm added a comment.
I just dug into how the ARC optimization pass is invoked... it really shouldn't
be invoked if `-disable-llvm-passes` is passed (or `-disable-llvm-optzns` which
appears to just alias the first). Can you verify that my command is sane:
/Users/danzimm/oss/build/bin/clang
danzimm added a comment.
In https://reviews.llvm.org/D41050#954668, @rjmccall wrote:
> In https://reviews.llvm.org/D41050#953917, @danzimm wrote:
>
> > Change tests to use non-O2 generated IR. It looks like the combined
> > objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls
>
rjmccall added a comment.
In https://reviews.llvm.org/D41050#953917, @danzimm wrote:
> Change tests to use non-O2 generated IR. It looks like the combined
> objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls
> annihilate each other and we just get a call/ret.
Is that really
danzimm added a comment.
Ah, just found test/Transforms/ObjCARC/rv.ll test3:
; Delete a redundant retainRV,autoreleaseRV when forwaring a call result
; directly to a return value.
; CHECK-LABEL: define i8* @test3(
; CHECK: call i8* @returner()
; CHECK-NEXT: ret i8* %call
define i8*
danzimm updated this revision to Diff 126763.
danzimm added a comment.
Change tests to use non-O2 generated IR. It looks like the combined
objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls annihilate
each other and we just get a call/ret.
Repository:
rC Clang
https://revi
rjmccall added a comment.
We do try to avoid relying on optimizer behavior in our tests. I don't know
why you need that here.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
danzimm updated this revision to Diff 126633.
danzimm added a comment.
Remove unnecessary change of braces
Repository:
rC Clang
https://reviews.llvm.org/D41050
Files:
lib/CodeGen/CGClass.cpp
test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
Index: test/CodeGenObjCXX/arc-forwarded-lambda-
danzimm added a comment.
@rjmccall aha, right - thanks for explaining that to me (sorry for the bad
logic I had earlier).
It turns out this was also broken for a lambda that was auto-converted to a
function pointer who returned an ObjC object. I changed the test case to
reflect the more genera
danzimm updated this revision to Diff 126628.
danzimm added a comment.
Call objc_retainAutoreleasedReturnValue after invoking a wrapped lambda
Repository:
rC Clang
https://reviews.llvm.org/D41050
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGClass.cpp
test/CodeGenObjCXX/arc-forwarded-l
rjmccall added a comment.
In https://reviews.llvm.org/D41050#952720, @danzimm wrote:
> Or do we need to abide by those semantics strictly here? Could you expand on
> why that is, if that's the case?
The autoreleased-return-value optimization cannot be understood purely in terms
of its impact
danzimm added a comment.
Or do we need to abide by those semantics strictly here? Could you expand on
why that is, if that's the case?
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
danzimm added a comment.
@rjmccall ah right, good point -- I think it's ok to elide that
objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue pair in this
case though, since all of this is generated within clang itself (and thus we
can make the guarentee that the object will properly
rjmccall added a comment.
Alternative #1 is actually the right option — only it shouldn't be an
objc_retain, it should be an objc_retainAutoreleasedReturnValue.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cf
gottesmm added a comment.
SGTM.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dexonsmith resigned from this revision.
dexonsmith added a comment.
Akira and/or John should take a look instead of me.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
gottesmm added a reviewer: dexonsmith.
gottesmm added a comment.
I do not work on objcarc any longer. +CC Duncan.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
trentxintong added a comment.
@gottesmm can you please take a look ? Thanks.
Repository:
rC Clang
https://reviews.llvm.org/D41050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danzimm created this revision.
Herald added subscribers: cfe-commits, kosarev.
Clang has a pretty cool feature right now that will allow you to use a lambda
as a block. Unfortunately there's a bug in this conversion if the return value
of the block is an ObjC object and arc is enabled -- the ret
26 matches
Mail list logo
