[clang] [llvm] [HLSL][DXIL][SPIRV] Create llvm dot intrinsic and use for HLSL (PR #102872)
Nikita Popov via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 16 11:57:45 PDT 2024
nikic wrote:
> > Please create a separate RFC for these intrinsics. I don't think there is a consensus on these intrinsics, and https://discourse.llvm.org/t/rfc-all-the-math-intrinsics/78294 covers way too much disparate ground (something like "tan" and something like "dot" are entirely separate categories, _especially_ if you also want to include integer intrinsics).
>
> @nikic The dot case was discussed pretty heavily in the rfc. Would you be open to moving forward if we drop the integer dot intrinsic as a target opcode, but keep the float dot product? We will also put an RFC together for the integer dot intrinsic cases.
There is some discussion in the RFC, but I don't see a consensus on the "dot" intrinsic in particular. I personally haven't found the arguments in favor of it particularly compelling.
This really needs an RFC specific to that intrinsic (class), which includes a clear definition of the semantics of the intrinsics, why it needs to be a intrinsic and how it maps to different hardware (in particular, whether the chosen definition is actually sufficiently portable).
Just looking at the LangRef wordings in this PR, your formulation is not appropriate for target-independent intrinsics. You cannot have a target-independent intrinsic "on a 2-4 element vector of 16-bit or 32-bit floating-point types". The description is also very vague for a floating-point operation. "The arguments are vectors to be elementwise multiplied and then summed" does not tell me *how* the summation occurs. Does it happen in order? As a tree reduction? In unspecified order? Does it use FMA?
Your definitions for sdot and udot also don't really make sense to me -- addition and multiplication are signedness-independent operations, so your sdot and udot are in fact equivalent. Separating these intrinsics only makes sense if they also involve a zero or sign extension, which your formulation does not -- but which would be necessary to support other targets, as pointed out above.
I'll also add that if you are adding target-independent intrinsics, the baseline expectation is that you need to provide full SDAG legalization support for them. I think we've been burned by this enough times that we're not going to accept new target-independent intrinsics that fail to do this.
https://github.com/llvm/llvm-project/pull/102872
More information about the cfe-commits
mailing list