Page MenuHomePhabricator

[clang][CodeGen] Factor out Swift ABI hooks (NFCI)
ClosedPublic

Authored by barannikov88 on Jul 22 2022, 1:03 PM.

Details

Summary

Swift calling conventions stands out in the way that they are lowered in
mostly target-independent manner, with very few customization points.
As such, swift-related methods of ABIInfo do not reference the rest of
ABIInfo and vice versa.
This change follows interface segregation principle; it removes
dependency of SwiftABIInfo on ABIInfo. Targets must now implement
SwiftABIInfo separately if they support Swift calling conventions.

Almost all targets implemented shouldPassIndirectly the same way. This
de-facto default implementation has been moved into the base class.

isSwiftErrorInRegister used to be virtual, now it is not. It didn't
accept any arguments which could have an effect on the returned value.
This is now a static property of the target ABI.

Diff Detail

Event Timeline

barannikov88 created this revision.Jul 22 2022, 1:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2022, 1:03 PM
Herald added a subscriber: dschuff. · View Herald Transcript
barannikov88 requested review of this revision.Jul 22 2022, 1:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2022, 1:03 PM

Reword commit message.

barannikov88 edited the summary of this revision. (Show Details)Jul 22 2022, 1:08 PM
barannikov88 retitled this revision from [clang][CodeGen] Factor out Swift ABI hooks to [clang][CodeGen] Factor out Swift ABI hooks (NFCI).Jul 22 2022, 1:17 PM

This seems like a reasonable set of changes, but I'd prefer someone more well-versed in codegen make the final call.

clang/lib/CodeGen/TargetInfo.h
63

SwiftInfo isn't always initialized to a nonnull pointer; should this have an assert to help point out when someone's forgotten to initialize that properly?

barannikov88 added inline comments.Aug 5 2022, 5:10 AM
clang/lib/CodeGen/TargetInfo.h
63

Good point, thank you

Assert that Swift ABI info is initialized before accessing it.

barannikov88 marked an inline comment as done.Aug 5 2022, 8:21 AM
rusyaev-roman accepted this revision.Aug 7 2022, 8:57 AM
rusyaev-roman added a subscriber: rusyaev-roman.

LGTM. Maybe in the future it's better to use SwiftABIInfo as mix-in like this

class ARMABIInfo : public ABIInfo, public SwiftABIInfo
This revision is now accepted and ready to land.Aug 7 2022, 8:57 AM
inclyc accepted this revision.Aug 7 2022, 9:20 AM

(Not a serious review) It seems like that your patch has already been accepted, so that I can commit this patch for you.

This revision was landed with ongoing or failed builds.Aug 7 2022, 9:23 AM
This revision was automatically updated to reflect the committed changes.

(Not a serious review) It seems like that your patch has already been accepted, so that I can commit this patch for you.

It was very nice of you, thanks!

inclyc added a comment.Aug 7 2022, 9:33 AM

I think in fact you are now fully qualified to apply for commit access. Don't worry, Lattner generally trusts all developers willing to contribute to LLVM.