Page MenuHomePhabricator

Convert some ObjC alloc msgSends to runtime calls
AcceptedPublic

Authored by pete on Dec 5 2018, 6:06 PM.

Details

Summary

It is faster to directly call the ObjC runtime for methods such as alloc/allocWithZone instead of sending a message to those functions.

This patch adds support for converting messages to alloc/allocWithZone to their equivalent runtime calls.

Tests included for the positive case of applying this transformation, negative tests that we ensure we only convert "alloc" to objc_alloc, not "alloc2", and also a driver test to ensure we enable this only for supported runtime versions.

Diff Detail

Event Timeline

pete created this revision.Dec 5 2018, 6:06 PM
rjmccall added inline comments.Dec 5 2018, 9:51 PM
include/clang/Basic/ObjCRuntime.h
189

"corresponding"

190

The actual code also rewrites allocWithZone: (which is why it's reasonable to pluralize "functions" in the method name).

200

Did we really add this so long ago? Wow.

lib/CodeGen/CGObjC.cpp
383

Using the word "selector" instead of "name" is both more technically accurate and nicely side-steps the difference between "alloc" (which you want to include) and "alloc:" (which you don't). You have this correct in the code.

pete updated this revision to Diff 177246.Dec 7 2018, 10:12 AM

Thanks for the review. Have fixed the comments as suggested.

pete marked 5 inline comments as done.Dec 7 2018, 10:12 AM
pete added inline comments.
include/clang/Basic/ObjCRuntime.h
200

Yeah! I was thinking that too!

rjmccall added inline comments.Dec 7 2018, 1:34 PM
lib/CodeGen/CGObjC.cpp
385

Actually, I'd suggest just rewriting this comment to look like the next one, it conveys the same idea but much more succinctly.

390

You should check !Args.front().hasLValue() here before calling getKnownRValue(). The test case would be some silly example where -allocWithZone: is declared with a parameter that's some big struct and the argument expression just loads it immediately from an l-value. Or you could just check that the parameter type is a pointer type.

pete updated this revision to Diff 177308.Dec 7 2018, 2:01 PM
pete marked an inline comment as done.

Added test for integer argument and updated code to only accept pointer types to allocWithZone.

rjmccall accepted this revision.Dec 7 2018, 2:03 PM

LGTM.

This revision is now accepted and ready to land.Dec 7 2018, 2:03 PM
pete added a comment.Dec 7 2018, 9:24 PM

LGTM.

Thanks for the review! Landed in r348687.