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

Implement the fmod HLSL Function #99118

Closed
Tracked by #99235
farzonl opened this issue Jul 16, 2024 · 7 comments · Fixed by #130320
Closed
Tracked by #99235

Implement the fmod HLSL Function #99118

farzonl opened this issue Jul 16, 2024 · 7 comments · Fixed by #130320
Assignees
Labels
backend:DirectX backend:SPIR-V bot:HLSL clang:headers Headers provided by Clang, e.g. for intrinsics 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

NOTE: SPIRV implementation is already completed.
NOTE: The change here shold be a header only changes with test case updates done to
clang/test/SemaHLSL/BuiltIns and clang/test/CodeGenHLSL/builtins/fmod.hlsl. The SPIRV tests should remain unchanged and continue to work.

fmod is currently defined as

_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
half fmod(half, half);
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
half2 fmod(half2, half2);
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
half3 fmod(half3, half3);
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
half4 fmod(half4, half4);

_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
float fmod(float, float);
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
float2 fmod(float2, float2);
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
float3 fmod(float3, float3);
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_fmod)
float4 fmod(float4, float4);

replace it with a templatized version

_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
const inline half fmod(half X, half Y) {
  return __detail::fmod_impl(X, Y);
}

const inline float fmod(float X, float Y) {
  return __detail::fmod_impl(X, Y);
}
template <int N, typename = std::enable_if_t<(N > 1 && N <=4)>>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
const inline vector<half, N> fmod(vector<half, N> X, vector<half, N> Y) {
  return __detail::fmod_vec_impl(X, Y);
}

template <int N, typename = std::enable_if_t<(N > 1 && N <=4)>>
const inline vector<float, N> fmod(vector<float, N> X, vector<float, N> Y) {
  return __detail::fmod_vec_impl(X, Y);
}

This should call a version in clang/lib/Headers/hlsl/hlsl_detail.h
that will do the target switching for us.

template <typename T>
constexpr enable_if_t<is_same<float, T>::value || is_same<half, T>::value, T>
fmod_impl(T X, T Y) {
#if !defined(__DirectX__)
  return __builtin_elementwise_fmod(X, Y));
#else 
  return /*insert algorithmic approach here*/;
#endif
}
template <typename T, int N>
constexpr vector<T, N> fmod_vec_impl(vector<T, N> X, vector<T, N> Y) {
#if !defined(__DirectX__)
  return __builtin_elementwise_fmod(X, Y));
#else 
  return /*insert algorithmic approach here*/;
#endif
}

DirectX

DXIL Opcode DXIL OpName Shader Model Shader Stages
6, 22 FAbs, Frc 6.0 ()

For reference the algorithm you are expected to implement in HLSL source shold match the DXC implementation.
Please find the reference implmentation linked here: https://github.com/microsoft/DirectXShaderCompiler/blob/c2ed9ad4ee775f3de903ce757c994aecc59a5306/lib/HLSL/HLOperationLower.cpp#L2253C1-L2271C2

  %1 = fdiv fast float %p1, %p2
  %2 = fsub fast float -0.000000e+00, %1
  %3 = fcmp fast oge float %1, %2
  %FAbs = call float @dx.op.unary.f32(i32 6, float %1)
  %Frc = call float @dx.op.unary.f32(i32 22, float %FAbs)
  %4 = fsub fast float -0.000000e+00, %Frc
  %5 = select i1 %3, float %Frc, float %4
  %6 = fmul fast float %5, %p2

SPIR-V

OpFRem:

Description:

The floating-point remainder whose sign matches the sign
of Operand 1.

Result Type must be a scalar or vector of floating-point
type
.

The types of Operand 1 and Operand 2 both must be the same as
Result Type.

Results are computed per component. The resulting value is undefined if
Operand 2 is 0. Otherwise, the result is the remainder
r of Operand 1 divided by Operand 2 where if r ≠ 0, the sign of
r is the same as the sign of Operand 1.

Word Count Opcode Results Operands

5

140

<id>
Result Type

Result <id>

<id>
Operand 1

<id>
Operand 2

Test Case(s)

Example 1

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

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

HLSL:

Returns the floating-point remainder of x/y.

ret fmod(x, y)

Parameters

Item Description
x
[in] The floating-point dividend.
y
[in] The floating-point divisor.

Return Value

The floating-point remainder of the x parameter divided by the y parameter.

Remarks

The floating-point remainder is calculated such that x = i * y + f, where i is an integer, f has the same sign as x, and the absolute value of f is less than the absolute value of y.

Type Description

Name Template Type Component Type Size
x scalar, vector, or matrix float any
y same as input x float same dimension(s) as input x
ret same as input x float same dimension(s) as input x

Minimum Shader Model

This function is supported in the following shader models.

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

Requirements

Requirement Value
Header
Corecrt_math.h

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
@lizhengxing
Copy link
Contributor

I'll work on this task.

lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 16, 2024
This change implements the frontend for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - add the builtin to DirectX intrinsic mapping
hlsl_intrinsics.h     - add the fmod api
SemaHLSL.cpp          - add type checks for builtin
IntrinsicsDirectX.td  - add the fmod DirectX intrinsic
@pow2clk pow2clk moved this to Active in HLSL Support Sep 23, 2024
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 24, 2024
This change implements the frontend for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaHLSL.cpp          - add type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 24, 2024
This change implements the frontend for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaHLSL.cpp          - add type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 24, 2024
This change implements the frontend for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaHLSL.cpp          - add type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
clang/docs/ReleaseNotes.rst        - announce the builtin
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 26, 2024
This change add the elementwise fmod builtin to support HLSL function 'fmod' in clang for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaChecking.cpp      - add type checks for builtin
SemaHLSL.cpp          - add HLSL type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
clang/docs/ReleaseNotes.rst        - announce the builtin
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 26, 2024
This change add the elementwise fmod builtin to support HLSL function 'fmod' in clang for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaChecking.cpp      - add type checks for builtin
SemaHLSL.cpp          - add HLSL type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
clang/docs/ReleaseNotes.rst        - announce the builtin
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 26, 2024
This change add the elementwise fmod builtin to support HLSL function 'fmod' in clang for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaChecking.cpp      - add type checks for builtin
SemaHLSL.cpp          - add HLSL type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
clang/docs/ReleaseNotes.rst        - announce the builtin
lizhengxing added a commit to lizhengxing/llvm-project that referenced this issue Sep 27, 2024
This change add the elementwise fmod builtin to support HLSL function 'fmod' in clang for llvm#99118
Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaChecking.cpp      - add type checks for builtin
SemaHLSL.cpp          - add HLSL type checks for builtin

clang/docs/LanguageExtensions.rst  - add the builtin in *Elementwise Builtins*
clang/docs/ReleaseNotes.rst        - announce the builtin
farzonl pushed a commit that referenced this issue Sep 27, 2024
This change add the elementwise fmod builtin to support HLSL function
'fmod' in clang for #99118

Builtins.td           - add the fmod builtin
CGBuiltin.cpp         - lower the builtin to llvm FRem instruction
hlsl_intrinsics.h     - add the fmod api
SemaChecking.cpp      - add type checks for builtin
SemaHLSL.cpp          - add HLSL type checks for builtin

clang/docs/LanguageExtensions.rst - add the builtin in *Elementwise
Builtins*
clang/docs/ReleaseNotes.rst        - announce the builtin
@damyanp
Copy link
Contributor

damyanp commented Oct 21, 2024

We want to decide llvm/wg-hlsl#86 before we continue with this.

@farzonl farzonl moved this from Active to Planning in HLSL Support Feb 4, 2025
@damyanp damyanp moved this from Planning to Ready in HLSL Support Feb 4, 2025
@damyanp damyanp moved this from Ready to Active in HLSL Support Feb 6, 2025
@kmpeng
Copy link
Contributor

kmpeng commented Feb 6, 2025

I'll be taking on this task.

@farzonl
Copy link
Member Author

farzonl commented Feb 27, 2025

@kmpeng @V-FEXrt had some concerns about vec1s. I noticed we aren't restricing vector sizes and we probably should.
My idea is template <int N, typename = std::enable_if_t<(N > 1 && N <=4)>>. The only thing I'm afraid of is what happens when we want to suport long vectors and we want to limit it to shader model 6.9.

My idea for thst is the following:

template <int N, typename = std::enable_if_t<(N > 1 && N <=4)>>
const inline vector<float, N> fmod(vector<float, N> X, vector<float, N> Y) {
  return __detail::fmod_vec_impl(X, Y);
}
_HLSL_AVAILABILITY(shadermodel, 6.9)
template <int N, typename = std::enable_if_t<(N >4)>>
const inline vector<float, N> fmod(vector<float, N> X, vector<float, N> Y) {
     return __detail::fmod_vec_impl(X, Y);
}

I filed a bug to fix up the other intrinsics here: #129003

@V-FEXrt
Copy link
Contributor

V-FEXrt commented Feb 27, 2025

@farzonl yeah we were talking about that yesterday. The expectation is that a vec1 would get cast/scalarized right?

@farzonl
Copy link
Member Author

farzonl commented Feb 27, 2025

@V-FEXrt not via this ticket. We haven’t supported vec1s for any of the non templatized intrinsics. We shouldn’t be adding it as an overload for fmod or any of the ones mentioned in the above mentioned bug. If they would get cast/scalarized/whatever that would be via overload resolution rules. We are probably screwing up those resolutions by having intrinsics that can take in vec1s

@V-FEXrt
Copy link
Contributor

V-FEXrt commented Feb 27, 2025

Got it, makes sense

@github-project-automation github-project-automation bot moved this from Active to Closed in HLSL Support Mar 12, 2025
@EugeneZelenko EugeneZelenko added the clang:headers Headers provided by Clang, e.g. for intrinsics label Mar 12, 2025
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 clang:headers Headers provided by Clang, e.g. for intrinsics 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.

6 participants