[PS4] handle dllimport/export w.r.t vtables/rtti

[bd1976llvm]

Summary

The existing Windows Itanium patches for dllimport/export behaviour w.r.t vtables/rtti can't be adopted for PS4 due to backwards compatibility reasons (see comments on https://reviews.llvm.org/D90299).

This review is for adding our PS4 scheme for this to Clang.

This is somewhat of a WIP. I am aware that some of these patches seem a bit awkwardly placed. Any assistance in how to refactor these patches would be extremely helpful.

[compnerd]

clang/include/clang/Basic/TargetInfo.h

1109

Please reflow the comment (or clang-format)

clang/lib/CodeGen/ItaniumCXXABI.cpp

1823

Can this not be const auto *D : RD->noload_decls()?

1824–1830

I think that the additional conditional and early break are a bit easier to read.

1903

I'm having a hard time following this comment.

I don't think that "Within the context of the Itanium C++ ABI, we want to mimic this MS behavior." adds any value.

What exactly does "defined in this TU/externally" mean? TU or defined externally means everywhere, no?

What is the "MS constraint"?

3156

Please reflow the text (or clang-format)

3799

I'm confused - does PS4 use a custom scheme or the MS ABI rules?

[bd1976llvm]

clang/lib/CodeGen/ItaniumCXXABI.cpp

1903

I will reword.

What is the "MS constraint"?

This is the constraint from the first sentence: "when dllimport/exporting only certain members of a class (rather than the non-inline virtual methods must be dllimported/exported, otherwise a link error will result". This constraint is documented here: https://docs.microsoft.com/en-us/cpp/cpp/using-dllimport-and-dllexport-in-cpp-classes?view=msvc-160#_pluslang_using_dllimport_and_dllexport_in_c2b2bselectivememberimportexport as "If you have a class in which you are using selective member import/export with virtual functions, the functions must be in the exportable interface or defined inline (visible to the client).".

What exactly does "defined in this TU/externally" mean? TU or defined externally means everywhere, no?

Sorry badly phrased, try: "For classes using selective member import/export: We dllimport the vtable if it is defined externally and all the non-inline virtual methods are marked dllimport. We dllexport the vtable if it is defined in this TU and all the non-inline virtual methods are marked dllexport. Otherwise the vtable is not imported or exported. With this scheme links succeed in circumstances where they would succeed for Microsoft toolchains and fail with a linker error (missing vtable for struct) in circumstances where MS toolchains would give a linker error.

Hope that makes it clearer. There is a wordier comment in the attached test explaining this.

3799

Sorry. Confusingly written :( The "custom scheme" I'm referring to is the patches in this review. The goal is to match the MS toolchain behaviour for dllimport/export on classes with vtables. This comment should probably just state: " We export the typeinfo in the same circumstances as the vtable is exported.".

[bd1976llvm]

Thanks for the suggestions. I have reworded the comments so that they are hopefully much clearer.

[bd1976llvm]

I have added this patch to https://reviews.llvm.org/D88124 along with some end to end tests to show the scei vendor windows-itanium behaviour. From the 'wi-test/test/dll_vtable_missing/test.py' test you can see that the link error given by the scei vendor variant is more consistent than the standard windows-itanium. Both variants should be equivalent in terms of runtime behaviour.