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

[SPIRV][HLSL] Implement dot lowering #88056

Closed
Tracked by #30
farzonl opened this issue Apr 8, 2024 · 3 comments · Fixed by #104656
Closed
Tracked by #30

[SPIRV][HLSL] Implement dot lowering #88056

farzonl opened this issue Apr 8, 2024 · 3 comments · Fixed by #104656

Comments

@farzonl
Copy link
Member

farzonl commented Apr 8, 2024

Add an intrinsic like this one from

to

def int_spv_all : DefaultAttrsIntrinsic<[llvm_i1_ty], [llvm_any_ty]>;

We won't need dot2,dot3,dot4 for spirv which adds a bit of a complication on our switching, Come with an intrinsic switching behavior that allows for per backend specializations. maybe as a per backend lambda?

update the dot.hlsl unit tests:

// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \

and implement the OpDot Lowering:
https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpDot

@farzonl farzonl added HLSL HLSL Language Support backend:SPIR-V labels Apr 8, 2024
@damyanp
Copy link
Contributor

damyanp commented Apr 16, 2024

Need resolution to #87367 before we can do this.

@farzonl
Copy link
Member Author

farzonl commented Jul 26, 2024

Note: #87367 was unblocked via https://discourse.llvm.org/t/rfc-all-the-math-intrinsics/78294.
The path forward was decided that dot would be a generic intrinsic. That should make the spirv side of this change trivial

if we did that we would likely want to move getDotProductIntrinsic to DXILIntrinsicExpansion.cpp and rewrite it as an expansion to replace the generic with the eqivalent dx dot2-4 intrinsic .

static bool expandDot(CallInst *Orig) {
  ...  
  Orig->replaceAllUsesWith(MaxCall);
  Orig->eraseFromParent();
}

Would also need a way of signallying that an intrinsic is needed for expansion:

static bool isIntrinsicExpansion(Function &F) {
  switch (F.getIntrinsicID()) {
  case Intrinsic::dot:
...
   return true;
  }
  return false;
}

One complication though would be we would need a generic vector dot operaton that will do element extractions, fmuls, and element insertions.

pow2clk added a commit to pow2clk/llvm-project that referenced this issue Aug 12, 2024
Use the new LLVM dot intrinsics to build SPIRV instructions.
This involves generating multiply and add operations for integers
and the existing OpDot operation for floating point. This includes
adding some generic opcodes for signed, unsigned and floats.
These require updating an existing test for all such opcodes.

New tests for generating SPIRV float and integer dot intrinsics are
added as well.

Fixes llvm#88056
pow2clk added a commit to pow2clk/llvm-project that referenced this issue Aug 16, 2024
All expansions end with replacing the previous inrinsic with the new
expansion and erasing the old one. By moving this operation to the
caller, these expansion functions can be called in more contexts
and a small amount of duplicated code is consolidated.

Pre-req for llvm#88056
pow2clk added a commit that referenced this issue Aug 17, 2024
…104626)

All expansions end with replacing the previous inrinsic with the new
expansion and erasing the old one. By moving this operation to the
caller, these expansion functions can be called in more contexts and a
small amount of duplicated code is consolidated.

Pre-req for #88056
pow2clk added a commit that referenced this issue Aug 22, 2024
This adds the SPIRV fdot, sdot, and udot intrinsics and allows them to
be created at codegen depending on the target architecture. This
required moving some of the DXIL-specific choices to DXIL instruction
expansion out of codegen and providing it with at a more generic fdot
intrinsic as well.

Removed some stale comments that gave the obsolete impression that type
conversions should be expected to match overloads.

The SPIRV intrinsic handling involves generating multiply and add
operations for integers and the existing OpDot operation for floating
point.

New tests for generating SPIRV float and integer dot intrinsics are
added as well as expanding HLSL tests to include SPIRV generation

Used new dot product intrinsic generation to implement normalize() in SPIRV

Incidentally changed existing dot intrinsic definitions to use
DefaultAttrsIntrinsic to match the newly added inrinsics

Fixes #88056
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 22, 2024

@llvm/issue-subscribers-clang-codegen

Author: Farzon Lotfi (farzonl)

Add an intrinsic like this one from https://github.com/llvm/llvm-project/blob/1e6ce5e284f5c0e8d64eee21af727bb164eb3caf/llvm/include/llvm/IR/IntrinsicsDirectX.td#L28

to

def int_spv_all : DefaultAttrsIntrinsic<[llvm_i1_ty], [llvm_any_ty]>;

We won't need dot2,dot3,dot4 for spirv which adds a bit of a complication on our switching, Come with an intrinsic switching behavior that allows for per backend specializations. maybe as a per backend lambda?

update the dot.hlsl unit tests:

// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \

and implement the OpDot Lowering:
https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpDot

cjdb pushed a commit to cjdb/llvm-project that referenced this issue Aug 23, 2024
This adds the SPIRV fdot, sdot, and udot intrinsics and allows them to
be created at codegen depending on the target architecture. This
required moving some of the DXIL-specific choices to DXIL instruction
expansion out of codegen and providing it with at a more generic fdot
intrinsic as well.

Removed some stale comments that gave the obsolete impression that type
conversions should be expected to match overloads.

The SPIRV intrinsic handling involves generating multiply and add
operations for integers and the existing OpDot operation for floating
point.

New tests for generating SPIRV float and integer dot intrinsics are
added as well as expanding HLSL tests to include SPIRV generation

Used new dot product intrinsic generation to implement normalize() in SPIRV

Incidentally changed existing dot intrinsic definitions to use
DefaultAttrsIntrinsic to match the newly added inrinsics

Fixes llvm#88056
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this issue Sep 2, 2024
This adds the SPIRV fdot, sdot, and udot intrinsics and allows them to
be created at codegen depending on the target architecture. This
required moving some of the DXIL-specific choices to DXIL instruction
expansion out of codegen and providing it with at a more generic fdot
intrinsic as well.

Removed some stale comments that gave the obsolete impression that type
conversions should be expected to match overloads.

The SPIRV intrinsic handling involves generating multiply and add
operations for integers and the existing OpDot operation for floating
point.

New tests for generating SPIRV float and integer dot intrinsics are
added as well as expanding HLSL tests to include SPIRV generation

Used new dot product intrinsic generation to implement normalize() in SPIRV

Incidentally changed existing dot intrinsic definitions to use
DefaultAttrsIntrinsic to match the newly added inrinsics

Fixes llvm#88056
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
5 participants