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

Incorrect specialization for modulo operator #5026

Closed
expipiplus1 opened this issue Sep 6, 2024 · 3 comments · Fixed by #5037
Closed

Incorrect specialization for modulo operator #5026

expipiplus1 opened this issue Sep 6, 2024 · 3 comments · Fixed by #5037
Assignees
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang kind:bug something doesn't work like it should

Comments

@expipiplus1
Copy link
Collaborator

expipiplus1 commented Sep 6, 2024

In this example, the % operator is being specialized as frem when it's actually operating on int, it should be translated to srem

//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=CHECK):-vk -output-using-type

// CHECK: type: int32_t
// CHECK-NEXT: 0
// CHECK-NEXT: 0
// CHECK-NEXT: 0
// CHECK-NEXT: 0

//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer
RWStructuredBuffer<int> outputBuffer;

T myMod<T : __BuiltinArithmeticType>(T x, T y)
{
    return x % y;
}

[numthreads(4, 1, 1)]
void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
{
    int c = 1;
    int d = 1;
    outputBuffer[dispatchThreadID.x] = myMod(c,d);
}
[readNone]
[nameHint("myMod")]
func %myMod     : Func(Int, Int, Int)
{
block %19(
                [nameHint("x")]
                param %x        : Int,
                [nameHint("y")]
                param %y        : Int):
        let  %20        : Int   = frem(%x, %y)
        return_val(%20)
}
@expipiplus1 expipiplus1 added the kind:bug something doesn't work like it should label Sep 6, 2024
@jkwak-work
Copy link
Collaborator

jkwak-work commented Sep 6, 2024

Isn't frem correct?
What is the expected behavior for this?

In HLSL, operator% is same as fmod, which returns a remainder not modulo.
Doesn't Slang follow the same rule of operator% in HLSL?

@jkwak-work
Copy link
Collaborator

I think this issue contradicts to one of already resolved issue.
#1059

@expipiplus1
Copy link
Collaborator Author

expipiplus1 commented Sep 6, 2024

Isn't frem correct?...

This doesn't have anything to do with the semantics of the operator, it's that we're failing to translate it correctly at all for integer types when used in a generic context, we should be lowering to SRem in this case, but we are lowering to FRem. Compare to just using the % operator not in a generic function where we correctly lower to SRem or UMod for signed and unsigned ints

@csyonghe csyonghe self-assigned this Sep 7, 2024
@csyonghe csyonghe added the goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang label Sep 7, 2024
@csyonghe csyonghe added this to the Q3 2024 (Summer) milestone Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang kind:bug something doesn't work like it should
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants