Skip to content

Implement the reflect HLSL Function #99152

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

Closed
12 tasks
Tracked by #99235
farzonl opened this issue Jul 16, 2024 · 8 comments · Fixed by #122992
Closed
12 tasks
Tracked by #99235

Implement the reflect HLSL Function #99152

farzonl opened this issue Jul 16, 2024 · 8 comments · Fixed by #122992
Assignees
Labels
backend:DirectX backend:SPIR-V bot:HLSL HLSL HLSL Language Support metabug Issue to collect references to a group of similar or related issues.

Comments

@farzonl
Copy link
Member

farzonl commented Jul 16, 2024

  • implement reflect in the hlsl_intrinsics.h
  • Implement reflect spirv target builtin in clang/include/clang/Basic/BuiltinsSPIRV.td
  • Add a spirv fast path in hlsl_detail.h in the form
#if (__has_builtin(__builtin_spirv_reflect))
  return __builtin_spirv_reflect(...);
#else
  return ...; // regular behavior
#endif
  • Add codegen for spirv reflect builtin to EmitSPIRVBuiltinExpr in CGBuiltin.cpp
  • Add HLSL codegen tests to clang/test/CodeGenHLSL/builtins/reflect.hlsl
  • Add SPIRV builtin codegen tests to clang/test/CodeGenSPIRV/Builtins/reflect.c
  • Add sema tests to clang/test/SemaHLSL/BuiltIns/reflect-errors.hlsl
  • Add spirv sema tests to clang/test/CodeGenSPIRV/Builtins/reflect-errors.c
  • Create the int_spv_reflect intrinsic in IntrinsicsSPIRV.td
  • In SPIRVInstructionSelector.cpp create the reflect lowering and map it to int_spv_reflect in SPIRVInstructionSelector::selectIntrinsic.
  • Create SPIR-V backend test case in llvm/test/CodeGen/SPIRV/hlsl-intrinsics/reflect.ll
  • Create SPIR-V backend test case in llvm/test/CodeGen/SPIRV/opencl/reflect.ll

DirectX

There were no DXIL opcodes found for reflect. That should make it easy to do entirely in the frontend.
In DXC it was done via emitting llvmir. We do not want to do that in upstream.

SPIR-V

Reflect:

Description:

Reflect

For the incident vector I and surface orientation N, the result is
the reflection direction:

I - 2 * dot(N, I) * N

N must already be normalized in order to achieve the desired result.

The operands must all be a scalar or vector whose component type is
floating-point.

Result Type and the type of all operands must be the same type.

Number Operand 1 Operand 2 Operand 3 Operand 4

71

<id>
I

<id>
N

Test Case(s)

Example 1

//dxc reflect_test.hlsl -T lib_6_8 -enable-16bit-types -O0

export float4 fn(float4 p1, float4 p2) {
    return reflect(p1, p2);
}

HLSL:

Returns a reflection vector using an incident ray and a surface normal.

ret reflect(i, n)

Parameters

Item Description
i
[in] A floating-point, incident vector.
n
[in] A floating-point, normal vector.

Return Value

A floating-point, reflection vector.

Remarks

This function calculates the reflection vector using the following formula: v = i - 2 * n * dot(i n) .

Type Description

Name Template Type Component Type Size
i vector float any
n vector float same dimension(s) as input i
ret vector float same dimension(s) as input i

Minimum Shader Model

This function is supported in the following shader models.

Shader Model Supported
Shader Model 1 (DirectX HLSL) and higher shader models yes

See also

Intrinsic Functions (DirectX HLSL)

@farzonl farzonl added backend:DirectX backend:SPIR-V bot:HLSL HLSL HLSL Language Support metabug Issue to collect references to a group of similar or related issues. labels Jul 16, 2024
@damyanp damyanp moved this to Ready in HLSL Support Oct 30, 2024
@damyanp damyanp moved this from Ready to Planning in HLSL Support Oct 30, 2024
@pow2clk pow2clk moved this from Planning to Designing in HLSL Support Nov 19, 2024
@jerryyiransun
Copy link
Contributor

Hello, is anyone currently working on this issue? If not, I’d like to take it on.

@farzonl
Copy link
Member Author

farzonl commented Dec 6, 2024

@jerryyiransun we are redesigning how we are going to do some of these intrinsics to be header only implementations. I’m on vacation right now. Give me two weeks and i’ll rewrite this ticket to match our expectations.

@jerryyiransun
Copy link
Contributor

@farzonl Thank you for the update, I'll wait for the updated ticket and proceed based on the revised expectations. Enjoy your vacation!

@farzonl farzonl moved this from Designing to Planning in HLSL Support Jan 9, 2025
@farzonl
Copy link
Member Author

farzonl commented Jan 9, 2025

This ticket is ready for planning.

@Icohedron
Copy link
Contributor

I will be working on this issue, if you don't mind @jerryyiransun

@jerryyiransun
Copy link
Contributor

@Icohedron of course, please go ahead.

@davidcook-msft davidcook-msft moved this from Planning to Active in HLSL Support Jan 14, 2025
inbelic pushed a commit that referenced this issue Jan 21, 2025
Fixes #99152

Tasks completed:

- Implement `reflect` in `clang/lib/Headers/hlsl/hlsl_intrinsics.h`
- Implement the `reflect` SPIR-V target built-in in
`clang/include/clang/Basic/BuiltinsSPIRV.td`
- Add a SPIR-V fast path in `clang/lib/Headers/hlsl/hlsl_detail.h` in
the form
 ```c++
#if (__has_builtin(__builtin_spirv_reflect))
  return __builtin_spirv_reflect(...);
 #else
   return ...; // regular behavior
 #endif
 ```
- Add codegen for the SPIR-V `reflect` built-in to
`EmitSPIRVBuiltinExpr` in `clang/lib/CodeGen/CGBuiltin.cpp`
- Add HLSL codegen tests to
`clang/test/CodeGenHLSL/builtins/reflect.hlsl`
- Add SPIR-V built-in codegen tests to
`clang/test/CodeGenSPIRV/Builtins/reflect.c`
- Add sema tests to `clang/test/SemaHLSL/BuiltIns/reflect-errors.hlsl`
- Add SPIR-V sema tests to
`clang/test/CodeGenSPIRV/Builtins/reflect-errors.c`
- Create the `int_spv_reflect` intrinsic in
`llvm/include/llvm/IR/IntrinsicsSPIRV.td`
- In `llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp` create the
`reflect` lowering and map it to `int_spv_reflect` in
`SPIRVInstructionSelector::selectIntrinsic`
- Create a SPIR-V backend test case in
`llvm/test/CodeGen/SPIRV/hlsl-intrinsics/reflect.ll`

Additional tasks completed:

- Implement sema check for the `reflect` SPIR-V built-in in
`clang/lib/Sema/SemaSPIRV.cpp`
- Required for HLSL codegen to work via the SPIR-V fast path, because
the types defined in `clang/include/clang/Basic/BuiltinsSPIRV.td` are
being overridden
- Create SPIR-V backend error test case in
`llvm/test/CodeGen/SPIRV/opencl/reflect-error.ll`
- Since `reflect` is only available in the GLSL extended instruction
set, using it in OpenCL should result in an error

Incomplete tasks:

- Create SPIR-V backend test case in
`llvm/test/CodeGen/SPIRV/opencl/reflect.ll`
- An OpenCL test is not applicable in this case because the [OpenCL
SPIR-V extended instruction
set](https://registry.khronos.org/SPIR-V/specs/unified1/OpenCL.ExtendedInstructionSet.100.html)
does not include a `reflect` function
@github-project-automation github-project-automation bot moved this from Active to Closed in HLSL Support Jan 21, 2025
@damyanp damyanp reopened this Jan 27, 2025
@damyanp damyanp moved this from Closed to Active in HLSL Support Jan 27, 2025
@damyanp
Copy link
Contributor

damyanp commented Feb 5, 2025

Blocked on #124045

@damyanp damyanp moved this from Active to Needs Review in HLSL Support Feb 11, 2025
@Icohedron
Copy link
Contributor

Closed since the PR has been relanded. (#125599)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:DirectX backend:SPIR-V bot:HLSL HLSL HLSL Language Support metabug Issue to collect references to a group of similar or related issues.
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

4 participants