Page MenuHomePhabricator

[X86] Lower calls with clang.arc.attachedcall bundle
Needs ReviewPublic

Authored by fhahn on Jan 13 2021, 6:11 AM.

Details

Summary

This patch adds support for lowering function calls with the
clang.arc.attachedcall bundle. The goal is to expand such calls to the
following sequence of instructions:

callq   @fn
movq  %rax, %rdi
callq   _objc_retainAutoreleasedReturnValue / _objc_unsafeClaimAutoreleasedReturnValue

This sequence of instructions triggers Objective-C runtime optimizations,
hence we want to ensure no instructions get moved in between them.
This patch achieves that by adding a new CALL_RVMARKER ISD node,
which gets turned into the CALL64_RVMARKER pseudo, which eventually gets
expanded into the sequence mentioned above.

The ObjC runtime function to call is determined by the
argument in the bundle, which is passed through as a
target constant to the pseudo.

@ahatanak is working on using this attribute in the front- & middle-end.

Together with the front- & middle-end changes, this should address
PR31925 for X86.

This is the X86 version of 46bc40e50246c1902a1ca7916c8286cb837643ee,
which added similar support for AArch64.

Diff Detail

Event Timeline

fhahn created this revision.Jan 13 2021, 6:11 AM
fhahn requested review of this revision.Jan 13 2021, 6:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2021, 6:11 AM
Herald added a subscriber: wdng. · View Herald Transcript
fhahn added a comment.Jan 13 2021, 6:20 AM

I am not too familiar with how calls are handled in the X86 backend. I would really appreciate any pointers to cases that this approach might miss.

This is only relevant for Objective-C code on X86, so it would probably be OK to drop support for the CALL32* variants?

RKSimon resigned from this revision.Jan 14 2021, 2:06 AM
RKSimon added a reviewer: pengfei.
skan added a subscriber: skan.Jan 18 2021, 6:07 PM
fhahn planned changes to this revision.Jan 21 2021, 1:28 AM

After checking with @ahatanak, we need a slightly different approach on X86 compared to ARM64. I'll update the patch shortly.

fhahn updated this revision to Diff 322138.Mon, Feb 8, 9:17 AM

Updated to work with operand bundles as available upstream, emit marker ( movq %rdi, %rax) followed by call to either _objc_retainAutoreleasedReturnValue or _objc_unsafeClaimAutoreleasedReturnValue.

fhahn updated this revision to Diff 322154.Mon, Feb 8, 10:08 AM

Fix marker, remove 32bit support, as we will only enable it for x86-64 in the frontend initially.

fhahn retitled this revision from [X86] Lower calls with rv_marker attribute. to [X86] Lower calls with clang.arc.rv bundle.Tue, Feb 9, 5:24 AM
fhahn edited the summary of this revision. (Show Details)
fhahn updated this revision to Diff 322992.Thu, Feb 11, 6:58 AM

Fix issue with pcrel32 calls, properly update RAX reg state (if it is implicit-def dead on the original call, we need to change it to implicit-def, because we add a new use; also set the state on the generated call); add reg mask for new call.

fhahn updated this revision to Diff 323937.Tue, Feb 16, 2:50 AM

Update bundle to clang.arc.attachedcall, in line with committed bundle name.

fhahn retitled this revision from [X86] Lower calls with clang.arc.rv bundle to [X86] Lower calls with clang.arc.attachedcall bundle.Tue, Feb 16, 2:50 AM
fhahn edited the summary of this revision. (Show Details)