This is an archive of the discontinued LLVM Phabricator instance.

[ObjC][ARC] Delete ObjC runtime calls if the argument is a global variable
ClosedPublic

Authored by ahatanak on May 24 2019, 3:48 PM.

Details

Summary

AFAIK, the only time clang emits runtime calls like objc_retain or objc_release on a global variable is when the object is a string literal or a global block and retaining or releasing them is a no-op since NSConstantString and __NSGlobalBlock are retain-agnostic. This patch teaches ARC optimizer to delete the calls when the call argument is a global variable.

rdar://problem/49839633

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak created this revision.May 24 2019, 3:48 PM

I thought we already did this optimization; you might want to talk to Dan or Michael about it.

It's a good optimization in the abstract.

I didn't find anything in the past commit log that mentions this optimization . Dan/Michael, do you know the reason ARC optimizer hasn't tried to remove these no-op calls?

I don't remember. That being said, this should be a conservative optimization. What if someone adds in the future a different global where this isn't a no-op. Is it possible if you know what the global is to add a whitelist that you only care about those 2 things.

ahatanak updated this revision to Diff 202815.Jun 3 2019, 4:11 PM

Check the presence of attribute "arc_retain_agnostic" on the global variable before deleting the ARC calls.

I don't remember. That being said, this should be a conservative optimization. What if someone adds in the future a different global where this isn't a no-op. Is it possible if you know what the global is to add a whitelist that you only care about those 2 things.

I agree we should be conservative here, so I made it check the presence of "arc_retain_agnostic" on the global. I'll post a clang patch that adds the attribute to the globals shortly.

Okay, that's fine with me.

I looked at the other one. Beyond my question about the name, this looks great!

This revision was not accepted when it landed; it landed in state Needs Review.Jun 14 2019, 3:05 PM
This revision was automatically updated to reflect the committed changes.