[cfe-dev] ARC + array literal patch feedback request

[John McCall]

On Mar 22, 2013, at 4:02 PM, Jesse Rusak <me at jesserusak.com> wrote:
> On 2013-03-22, at 6:45 PM, John McCall <rjmccall at apple.com> wrote:
>> On Mar 20, 2013, at 10:41 AM, Jesse Rusak <me at jesserusak.com> wrote:
>>> Thanks for the detailed response; this makes a lot of sense to me. So, clang.arc.use_objects is a non-existent function which is removed after the main ARC optimizer runs, but the use of those objects is sufficient to prevent ARC from moving the release up. I'll try this out and put together a patch.
>> 
>> I've actually done the LLVM side of this as r177769, although the function is named @clang.arc.use now.
> 
> Ah, great. I had something similar coming together, but I had two concerns that I was still working through:
> 
> First, I think the ARC contract phase only runs under -O1 or higher, so calls to @clang.arc.use will remain under -O0 and cause link-time errors. I was thinking of either changing EmitAssemblyHelper::AddEmitPasses to always include the contract pass, or else move the erasure logic elsewhere (perhaps to a new, later ARC pass).

Oh, you're underestimating how hacky the interaction between IR-gen and the optimizer is here. :)  The ARC contract pass is essentially an ARC-specific codegen prepare pass that cleans up things that we want the optimizer to see (or not see).  For example, it inserts the objc_retainAutoreleaseReturnValue assembly markers, which the frontend is responsible for at -O0 but which would really annoy/disable the optimizer if we emitted them at -O1 or higher.

So the solution is to just not emit these intrinsic calls at -O0.

> Second, I was also concerned with the interpreter, which seems to use IntrinsicLowering to evaluate intrinsics; do we need to add some logic to IntrinsicLowering::LowerIntrinsicCall to handle this function?

The interpreter needs to be either running -O0 code without optimization or queuing up the ARC contract pass.  I actually don't know what it's currently doing.

> The other option which might solve both of these is to make this a "real" LLVM intrinsic and give it an empty body.

Such an intrinsic would probably promise too much — we don't actually need these values to be live across the call in the general compiler sense.  We really do want ARC-specific semantics here.

> Let me know if you have any thoughts about the above; I was planning on finishing up a first patch this weekend. 


I'm actually going to land a patch for a different but similar miscompile soon — this one with the writeback-from-out-parameter operation.  So the frontend will have the basic infrastructure for this already.

John.