Page MenuHomePhabricator

Change the objc ARC optimizer to use the new objc.* intrinsics
ClosedPublic

Authored by pete on Dec 5 2018, 5:26 PM.

Details

Summary

Now that we have objc* intrinsics, this teaches ARC to use them instead of the objc_ methods.

The vast majority of the changes are in the tests where I really just replaced @objc_ with @llvm.objc.

In the ARC optimizer itself, the changes are to both use the new intrinsics as the source to optimize, and also, where necessary, create calls to the intrinsics instead of the objc_ methods.

Diff Detail

Repository
rL LLVM

Event Timeline

pete created this revision.Dec 5 2018, 5:26 PM
dexonsmith added inline comments.Dec 5 2018, 7:51 PM
include/llvm/Analysis/ObjCARCAnalysisUtils.h
58 ↗(On Diff #176910)

I'm curious why you didn't create an intrinsic for this method (or the two at the bottom). Are they not emitted by Clang's IRGen?

lib/Analysis/ObjCARCInstKind.cpp
154 ↗(On Diff #176910)

Why is this okay to leave behind?

pete marked 2 inline comments as done.Dec 6 2018, 10:22 AM
pete added inline comments.
include/llvm/Analysis/ObjCARCAnalysisUtils.h
58 ↗(On Diff #176910)

Ah yeah, I found that out later that they are emitted by clang.

I only added the ones documented at https://clang.llvm.org/docs/AutomaticReferenceCounting.html#runtime-support

Its very easy to add the others though. Can do that now if you like?

lib/Analysis/ObjCARCInstKind.cpp
154 ↗(On Diff #176910)

Same as above I believe. These are not yet intrinsics, but could be if we want completeness here. It would certainly make this method much simpler if they are intrinsics.

pete updated this revision to Diff 177296.Dec 7 2018, 1:35 PM

I added the other objc* intrinsics used by ARC in r348646.

Updated this patch to use them so now llvm::objcarc::GetFunctionClass is all intrinsic based.

pete marked 2 inline comments as done.Dec 7 2018, 1:36 PM

I added the other objc* intrinsics used by ARC in r348646.

Updated this patch to use them so now llvm::objcarc::GetFunctionClass is all intrinsic based.

Thanks! I don't see anything else, but I don't know the ARC optimizer well. @ahatanak, can you take a look?

ahatanak accepted this revision.Dec 9 2018, 11:19 PM

LGTM

lib/Analysis/ObjCARCInstKind.cpp
133 ↗(On Diff #177296)

It looks like these functions have been deprecated.

https://opensource.apple.com/source/objc4/objc4-532/runtime/objc.h

lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h
92 ↗(On Diff #177296)

Indentation looks incorrect.

98 ↗(On Diff #177296)

Indentation looks incorrect.

test/Transforms/ObjCARC/allocas.ll
12 ↗(On Diff #177296)

Is there a reason we don't use @llvm.objc.retainedObject here and in test/Transforms/ObjCARC/rv.ll?

This revision is now accepted and ready to land.Dec 9 2018, 11:19 PM
pete marked 6 inline comments as done.Dec 17 2018, 5:57 PM
pete added inline comments.
lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h
92 ↗(On Diff #177296)

Ah yeah, 80 cols so had to do it this way :(

98 ↗(On Diff #177296)

Also 80 cols.

test/Transforms/ObjCARC/allocas.ll
12 ↗(On Diff #177296)

Good catch!

Looks like those are declared but unused in both tests. I'll remove them in a followup.

This revision was automatically updated to reflect the committed changes.
pete marked 3 inline comments as done.