Skip to content

Conversation

wenju-he
Copy link
Contributor

@wenju-he wenju-he commented Sep 2, 2025

_CLC_DEF_WEAK can be used in our downstream libclc to allow overriding generic __clc_tgamma implementation.

_CLC_DEF_WEAK can be used in our downstream libclc to allow overriding
generic __clc_tgamma implementation.
@wenju-he wenju-he requested a review from frasercrmck September 2, 2025 00:29
@llvmbot llvmbot added the libclc libclc OpenCL library label Sep 2, 2025
Copy link
Contributor

@frasercrmck frasercrmck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

My main question is - why not make all definitions weak? The alternative is to make them weak as and when we or a downstream needs them, or come up with some rationale for which ones are or aren't weak.

@wenju-he
Copy link
Contributor Author

wenju-he commented Sep 3, 2025

My main question is - why not make all definitions weak? The alternative is to make them weak as and when we or a downstream needs them, or come up with some rationale for which ones are or aren't weak.

Do you mean make all generic definitions weak? That sounds like a good idea.
We can't make non-generic definitions weak since it is not clear whether generic or non-generic variant with the same name is kept after linking, right?

@wenju-he
Copy link
Contributor Author

wenju-he commented Sep 3, 2025

Unfortunately weak linkage prevents PostOrderFunctionAttrsPass from deducing attributes for functions that have weak linkage:

checkFunctionMemoryAccess(*F, F->hasExactDefinition(), AAR, SCCNodes);
where hasExactDefinition returns false for weak linkage.

weak_odr probably suits libclc and it can enables optimization in PostOrderFunctionAttrsPass. However, there is no corresponding clang attribute, i.e. __attribute__(weak_odr) isn't available.

edit: weak_odr doesn't work either since hasExactDefinition returns false for weak_odr linkage.

@wenju-he
Copy link
Contributor Author

wenju-he commented Sep 3, 2025

Applying weak attribute to a small set of libclc functions, like this PR does, might be acceptable because libclc will eventually set linkage to linkonce_odr for all functions. The small set of libclc functions, that were not optimized by PostOrderFunctionAttrsPass during libclc build, will be optimized by the same pass after being linked into user code.

@wenju-he
Copy link
Contributor Author

wenju-he commented Sep 4, 2025

@frasercrmck I proposed an alternative approach that overrides generic symbol using llvm-link --override flag instead of using weak linkage: #156778
Close this PR.

@wenju-he wenju-he closed this Sep 4, 2025
@wenju-he wenju-he deleted the __CLC_DEF_weak branch September 4, 2025 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libclc libclc OpenCL library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants