Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HLSL][DXIL][SPIRV] Intrinsic unification PR #87034

Closed
wants to merge 1 commit into from

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented Mar 29, 2024

**** DO NOT MERGE ****
This is part of a proposal for how to unify spir-v and DirectX intrinsics.
The issue tracking this work is: #83882

@bogner
Copy link
Contributor

bogner commented Mar 29, 2024

I have some misgivings about abusing the target intrinsic infrastructure like this to create "target" intrinsics for a non-existent target. IMO this is morally equivalent to just putting all of these intrinsics as generic llvm intrinsics (llvm.thread_id or llvm.all), just with some extra namespacing.

For a long time there has been some appetite for being able to strip the target intrinsics for targets you aren't building (see this very old post or the commit that split up intrinsic headers per-target). I worry that using the target infrastructure but keyed on something other than a target is a step in the wrong direction there.

The other concern is just about layering. Today, backends need to handle generic LLVM intrinsics and their own intrinsics only. This is introducing intrinsics that apply across an arbitrary set of backends (DirectX and SPIRV today, possibly some GPU or otherwise SIMT backends in the future...). What's the contract for who needs to handle them? Can we encode that contract in a way that isn't simply best effort as new intrinsics are added? As is, this approach makes those problems harder rather than easier.

@farzonl farzonl closed this Mar 29, 2024
@farzonl farzonl deleted the hlsl-spirv-lowering-unification branch March 29, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants