Skip to content

Conversation

wenju-he
Copy link
Contributor

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

Before this PR, weak linkage is applied to a few CLC generic functions to allow target specific implementation to override generic one. However, adding weak linkage has a side effect of preventing inter-procedural optimization, such as PostOrderFunctionAttrsPass, because weak function doesn't have exact definition (as determined by hasExactDefinition in the pass).

This PR resolves the issue by adding --override flag for every non-generic bitcode file in llvm-link run. This approach eliminates the need for weak linkage while still allowing target-specific implementation to override generic one.
llvm-diff shows imporoved attribute deduction for some functions in amdgcn--amdhsa.bc, e.g.
%23 = tail call half @llvm.sqrt.f16(half %22)
=>
%23 = tail call noundef half @llvm.sqrt.f16(half %22)

…ead of using weak linkage

Before this PR, weak linkage is applied to a few CLC generic functions
to allow target specific implementation to override generic
one. However, adding weak linkage has a side effect of preventing
inter-procedural optimization, such as PostOrderFunctionAttrsPass,
because weak function doesn't have exact definition (as determined by
hasExactDefinition in the pass).

This PR resolves the issue by adding --override flag for every
non-generic bitcode file in llvm-link run. This approach eliminates
the need for weak linkage while still allowing target-specific
implementation to override generic one.
llvm-diff shows imporoved attribute deduction for some functions in
amdgcn--amdhsa.bc, e.g.
  %23 = tail call half @llvm.sqrt.f16(half %22)
=>
  %23 = tail call noundef half @llvm.sqrt.f16(half %22)
Copy link

github-actions bot commented Sep 4, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

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.

Nice find, thanks.

It might mean it's harder to move away from using LLVM tools (like llvm-link) and towards something more like the other runtimes libraries do (using clang), though. I don't know if that's on the roadmap or not.

@wenju-he
Copy link
Contributor Author

wenju-he commented Sep 4, 2025

It might mean it's harder to move away from using LLVM tools (like llvm-link) and towards something more like the other runtimes libraries do (using clang), though. I don't know if that's on the roadmap or not.

Good question. IIUC that means in the future we could use thin-lto for libclc compilation, right?

I think --internalize flag, which is currently required to internalize symbols in CLC library, has similar problem. We can investigate how to mitigate the issues later.

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.

4 participants