Default Function Linkage #254
Conversation
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ durign CodeGen phase in case the function does not have a body. | ||
|
|
||
| 2. Assign _internal linkage_ to functions that are guaranteed to stay _internal_ in the final DXIL, such as the functions in unnamed namespace. For all other functions assume _external linkage_ for Clang semantic analysis and change it to _internal linkage_ in CodeGen phase for non-entry and non-export function definitions. | ||
|
|
There was a problem hiding this comment.
Option no.2 is currently in review in PR llvm/llvm-project#93336.
There was a problem hiding this comment.
I'd really like a full list of functions guaranteed to stay internal. I have a clear idea of what is sure to be external, but not what is sure to be internal.
|
|
||
| In order to implement DXC behavior there are two options: | ||
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ durign CodeGen phase in case the function does not have a body. | ||
|
|
There was a problem hiding this comment.
I'll prototype this to see how it might work.
There was a problem hiding this comment.
Draft PR with option no.1: llvm/llvm-project#95331
There was a problem hiding this comment.
... change it to external linkage during CodeGen phase in case the function does not have a body.
Changing the linkage of functions without a body appears to denote an implicit / underspecified way to declare external functions.
Would the use of extern keyword to specify such functions that appear to be implicitly considered to have external linkage, be a good and unambiguous language clarification option? It appears that extern is supported in HLSL but only applies to variable declaration. Extending this usage to functions appears to provide a clear / explicit mechanism. Doing so would also obviate Option 2 below and the slightly different implicit assumptions.
There was a problem hiding this comment.
The extern keyword on variable declarations is implied by default and effectively ignored. For marking functions to be exported from shader library there is export keyword (llvm/llvm-project#92812).
There was a problem hiding this comment.
Consider example below. This currently works is DXC (-T lib_6_6):
void f();
export void f() {
}
If we want to assign the correct linkage of f in Clang and not change it later in CodeGen, and the default linkage of un-decorated functions is internal, we will need to start requiring export on the f declaration to make this work.
There was a problem hiding this comment.
Similarly for shaders entry functions. This works is DXC (-T lib_6_6):
void CSMain();
[shader("compute")]
[numthreads(4,1,1)]
void CSMain() {
}
In Clang the CSMain declaration might or might not need [shader("compute")]. We might be able to avoid it for shader entry points because currently in CodeGen we are already making the shader entry function internal and wrapping in an external function that calls it.
There was a problem hiding this comment.
Regarding non-library target compilation of HLSL: I presume there is an expectation that function declarations with no internal linkage - and changed to have external linkage in the current DXC implementation - to be resolved by the driver compiler, similar to a linking loader that adheres to the C-like language compilation linkage models. If so, using some keyword (such as extern) to indicate that intent in the sources and emitting an error otherwise, seems unambiguous. It would potentially be in line with C-language compilation linkage model and play well with Clang assumptions (not an expert on the Clang implementation, though). That would mean breaking from the implicit promotion to external of function declarations with no definitions, as done in current DXC (IIUC). Not sure if there is a strong reason to retain the current DXC model.
There was a problem hiding this comment.
I read most of this thinking about stage compilations. Perhaps that is not relevant at all to this document. If so, you can discard all of my comments on the subject, but I think it should be made clear early that this is only concerning library compilation.
I'm not sure this is a language spec as it concerns how the infrastructure compiles things rather than any changes to the language or even definition of how existing keywords function except insofar as they affect this functionality. I think this might be an infrastructure spec, which would involve different naming and a slightly different template.
| * Author(s): [Helena Kotas](https://github.com/hekota) | ||
| * Sponsor: [Helena Kotas](https://github.com/hekota) | ||
| * Status: **Under Consideration** | ||
| * Proposed Version: 202y (Clang-only) |
There was a problem hiding this comment.
If this is an infrastructure spec, the current infrastructure template has "impacted projects" here instead of "proposed version". If not, it recommends omitting "Planned Version" until the review.
| ## Introduction | ||
|
|
||
| This document maps the current default linkage of functions in DXC and discusses how it implementat it in Clang. | ||
|
|
There was a problem hiding this comment.
The template requests a "Motivation" section here
|
|
||
| This document maps the current default linkage of functions in DXC and discusses how it implementat it in Clang. | ||
|
|
||
| ## Existing Behavior in DXC |
There was a problem hiding this comment.
I think from here on would match the "Proposed Solution" section that the template suggests.
|
|
||
| ## Existing Behavior in DXC | ||
|
|
||
| In DXC today functions have internal linkage by default unless they are shader entry point functions or library export functions marked with an `export` keyword. |
There was a problem hiding this comment.
I think we need to clarify what we mean by "entry point functions". I think that encompasses stage compilations where the active entry point is specified and also library compilations where the shader[""] annotation identifies the functions that are effectively exported by default.
What I can't decide is whether mentioning the poor patch constant function here is worth it. It behaves very much like an active entry function for these purposes.
|
|
||
| HLSL functions that are not entry points or exported are most likely going to be inlined. This can be prevented by disabling optimizations, making sure the function is called and putting `[noinline]` attribute on the function, and the generated DXIL function it will have an _internal linkage_ and a _mangled_ name. | ||
|
|
||
| However, if such function is just declared without a definition, the generated DXIL will include the function declaration with _external linkage_ and _mangled_ name: |
There was a problem hiding this comment.
That's only true if it's called and if you're compiling to a library. In that case, it's treated much like an exported function. If you're compiling to a stage and it has a use, you get an error. If it's not called, it gets eliminated.
|
|
||
|
|
||
| ``` | ||
| [noinline] |
There was a problem hiding this comment.
I don't think we should ignore noinline entirely, but the way it works and doesn't now and how it might in the future is sufficiently unsure that I agreed to mostly leave it unmentioned in my spec. I was thinking the mention above is fine, but this example implies promises I'm not sure we can keep
|
|
||
| In Clang it is assumed linkage of functions can be determined during semantic analysis and that it does not change. The linkage is calculated based on many parameters and the results are cached for performance reasons. | ||
|
|
||
| Identifying a shader entry point or exported function during parsing is straightforward and such function can be assigned correct linkage right from the beginning. |
There was a problem hiding this comment.
For stage compilations and entry functions, it has at least as much to do with the specified entry point name as parsing
| However, in case of functions that are not entry points or exported, if we want to keep on par with DXC, the final linkage cannot be determined until the whole translation unit is parsed and it is known whether the function is defined (has a body) or if it is just a declaration. | ||
|
|
||
| In order to implement DXC behavior there are two options: | ||
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ durign CodeGen phase in case the function does not have a body. |
There was a problem hiding this comment.
typo:
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ durign CodeGen phase in case the function does not have a body. | |
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ during CodeGen phase in case the function does not have a body. | |
There was a problem hiding this comment.
Should also add the detail "does not have a body and is called" as we eliminate it otherwise.
| @@ -0,0 +1,98 @@ | |||
| # Default Function Linkage | |||
|
|
|||
| * Proposal: [0021](0021-function-linkage.md) | |||
There was a problem hiding this comment.
Should this have a number already? Initial submissions usually have NNNN
| However, in case of functions that are not entry points or exported, if we want to keep on par with DXC, the final linkage cannot be determined until the whole translation unit is parsed and it is known whether the function is defined (has a body) or if it is just a declaration. | ||
|
|
||
| In order to implement DXC behavior there are two options: | ||
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ durign CodeGen phase in case the function does not have a body. |
There was a problem hiding this comment.
Should also add the detail "does not have a body and is called" as we eliminate it otherwise.
| 1. Assign _internal linkage_ to non-entry and non-export functions by default while in Clang semantic analysis phase and change it to _external linkage_ durign CodeGen phase in case the function does not have a body. | ||
|
|
||
| 2. Assign _internal linkage_ to functions that are guaranteed to stay _internal_ in the final DXIL, such as the functions in unnamed namespace. For all other functions assume _external linkage_ for Clang semantic analysis and change it to _internal linkage_ in CodeGen phase for non-entry and non-export function definitions. | ||
|
|
There was a problem hiding this comment.
I'd really like a full list of functions guaranteed to stay internal. I have a clear idea of what is sure to be external, but not what is sure to be internal.
This document maps the current default function linkage in DXC and opens up the discussion on how to implement it in Clang.. Links to two implementation options are presented below.
I got some feedback on the first PR which I am not sure how much I should be concerned about. So I wanted document what DXC does today, what are the option on implementing it in Clang, and get some more eyes on it. Please let me know what you think!
Related PR: llvm/llvm-project#93336