Cleanup uses of SWIFT_RUNTIME_EXPORT in implementation files

[hughbe]

• And remove duplicated SWIFT_CC(swift) annotations.

See discussion in #7103 and #6820

I have some questions about the SWIFT_CC macro:

• what does it do?
• why is it declared as SWIFT_CC(DefaultCC) in the header, and SWIFT_CC(DefaultCC_Impl) in the implementation? What does this achieve? Can we clean them up?

[hughbe]

@swift-ci please smoke test

[gparker42]

SWIFT_CC changes the CPU calling convention for a function. On some platforms, some internal calls use a different calling convention for performance reasons. (It's comparable to __cdecl vs __stdcall if you're familiar with low-level Windows details.)

@hughbe, can you separate the SWIFT_CC changes here? The SWIFT_RUNTIME_EXPORT changes are definitely fine, but I don't know about the rules for SWIFT_CC.

@swiftix, can you comment on IMPL vs not-IMPL in SWIFT_CC? I don't see any distinction in their current definitions.

[swiftix]

@hughbe You should definitely not remove SWIFT_CC(swift) anywhere, because that would change the calling convention on some platforms.

As for the distinction between IMPL and not-IMPL, IIRC (though I'm not 100% sure about it anymore ;-), the idea was to mark declarations of runtime functions with SWIFT_CC(DefaultCC) and implementations with SWIFT_CC(DefaultCC_IMPL), so that one could easily redefine calling conventions should it be needed. I'd suggest keeping them as is for the time being.

[gparker42]

Is that _IMPL idea actually used consistently? The changes proposed here show two cases where a non-IMPL CC is used on a implementation.

[swiftix]

@gparker42 Are you taking about swift_allocBox and swift_getTypeName? I just checked. It was added by @rjmccall. It looks like he has not realized this pattern, i.e. that implementations should use the _IMPL prefix. We should fix it to be more consistent. And we should add a comment to include/Runtime/Config.h describing the difference, so that we do not forget in the future.

[gparker42]

Is this distinction used anywhere today? If there is not then we should not attempt to do it, because it is unthinkable that it will actually be correct. If we need to separately annotate declaration and definition in the future then we can re-add them later.

[swiftix]

@gparker42 Well, in ideal world, the single point of truth should be the RuntimeFunctions.def and the calling convention and parameters parts of those declarations/definitions of functions in the runtime should be auto-generated from it. This would ensure the consistency between what IRGen assumes about the runtime functions and the way how they are implemented by the runtime. BTW, there is even a radar about it and it is assigned to you ;-)

But coming back to IMPL and not-IMPL. IIRC, it is not currently used anywhere. It was foreseen for the future. So, we could remove the distinction. But if this will be done, it should be done in a separate PR.

[hughbe]

Thanks @swiftix

@hughbe You should definitely not remove SWIFT_CC(swift) anywhere, because that would change the calling convention on some platforms.

The reason I removed these SWIFT_CC(swift) annotations is because they are already included in the header definition. I'm under the impression that, similarly to SWIFT_RUNTIME_EXPORT, we don't need to duplicate the SWIFT_CC annotation in the header and implementation - only the header is necessary. Am i wrong?

That said, I've reverted the removal of SWIFT_CC(swift) pending an answer to my question above. If these duplicate SWIFT_CC annotations are indeed redundant, I'll clean them up in a future PR.

But coming back to IMPL and not-IMPL. IIRC, it is not currently used anywhere. It was foreseen for the future. So, we could remove the distinction. But if this will be done, it should be done in a separate PR.

Okay. I'll submit a PR in the future either removing the SWIFT_CC(*_IMPL) or changing it to SWIFT_CC(*) depending on the answer to my question above.

[hughbe]

@swift-ci please smoke test

[hughbe]

@swift-ci please smoke test

[hughbe]

@swift-ci please smoke test

[swiftix]

The reason I removed these SWIFT_CC(swift) annotations is because they are already included in the header definition. I'm under the impression that, similarly to SWIFT_RUNTIME_EXPORT, we don't need to duplicate the SWIFT_CC annotation in the header and implementation - only the header is necessary. Am i wrong?

I don't remember for sure why I duplicated the annotations. IIRC, there was an issue with clang that in a situation where a declaration and definition of the same function would use different calling conventions clang would not always pick the right one. I'd suggest that you simply try out what happens if you remove any calling convention specifiers from the definitions. Temporarily define the calling convention to map to a non-standard calling convention, e.g. preserve_most. Check if the generated assembly stores/restores the registers according to the specified calling convention. Or you could try to produce the LLVM IR and see if the functions in question have a proper calling convention attribute. If we see that the calling convention is properly used, then we can safely remove the calling conventions specifiers from the definitions. Does it sound like a plan?

[gparker42]

@swift-ci please smoke test OS X