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

Arithmetic and conditional errors with (byte) casts #772

Closed
Ruberik opened this issue Mar 18, 2022 · 2 comments · Fixed by #774
Closed

Arithmetic and conditional errors with (byte) casts #772

Ruberik opened this issue Mar 18, 2022 · 2 comments · Fixed by #774
Labels
Milestone

Comments

@Ruberik
Copy link
Contributor

Ruberik commented Mar 18, 2022

Summary

Casting uints to bytes sometimes causes unexpected behaviour in simple arithmetic and conditional operations. This appears to happen only when the uint in question has its 128 bit set. The output depends on whether it's being built in Debug or Release mode. This has been tested on at least a T4, and with both rc1 and older ILGPU code.

Code

public class MiscClean {
    public static void KernelByteBugs(Index1D index, ArrayView1D<uint, Stride1D.Dense> input, ArrayView1D<uint, Stride1D.Dense> output) {
        for (int i = 0; i < input.Length; i++) {
            byte b = (byte)input[i];
            output[0] += b;
            output[1] += ((byte)input[i]) & 255u;
            output[2] += ((byte)input[i]);
            output[3] += input[i] & 255;
            byte b0 = (byte)(input[i]);
            if (b0 < 128) output[4]++;
            if (((input[i]) & 255) < 128) output[5]++;
        }
    }

    static void Main(string[] args) {
        using var context = Context.Create().Cuda().ToContext();
        using var acc = context.CreateCudaAccelerator(0);
        var input = new uint[1] { 130u };
        var output = new uint[6];
        var gpuInput = acc.Allocate1D(input);
        var gpuOutput = acc.Allocate1D(output);
        acc.LaunchAutoGrouped(KernelByteBugs, new Index1D(1), gpuInput.View, gpuOutput.View);
        gpuOutput.CopyToCPU(output);
        for (int i = 0; i < output.Length; i++) {
            Console.WriteLine($"output[{i}] = {output[i]}");
        }
    }
}
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <ApplicationIcon />
    <OutputType>Exe</OutputType>
    <StartupObject />
    <RootNamespace>FurrowUtilities</RootNamespace>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="ILGPU" Version="1.1.0-rc1" />
    <PackageReference Include="ILGPU.Algorithms" Version="1.1.0-rc1" />
    <PackageReference Include="System.Collections.Immutable" Version="6.0.0" />
  </ItemGroup>
</Project>

Output

Expected:

output[0] = 130
output[1] = 130
output[2] = 130
output[3] = 130
output[4] = 0
output[5] = 0

Actual, in debug mode:

output[0] = 130
output[1] = 130
output[2] = 4294967170
output[3] = 130
output[4] = 0
output[5] = 0

Actual, in release mode:

output[0] = 130
output[1] = 130
output[2] = 4294967170
output[3] = 130
output[4] = 1
output[5] = 0

Note the difference on output[4].

Notes

  1. 4294967170 = 0xFFFFFF82 (where 0x82 = 130).
  2. The hard-coded value=130 is important: this issue apparently happens iff (value & 128) != 0. Behaviour is as expected if that bit isn't set.
@MoFtZ
Copy link
Collaborator

MoFtZ commented Mar 20, 2022

@Ruberik I have raised a PR that fixes this issue.

@m4rs-mt The issue appears to be that nested conversions are ignoring the unsigned flag of the inner conversion. However, I could only replicate this on byte to uint. Is there something that I am missing?

@Ruberik
Copy link
Contributor Author

Ruberik commented Mar 21, 2022

Thanks, @MoFtZ. I tested on my machine and can confirm this apparently fixes the problem.

@m4rs-mt m4rs-mt added this to the v1.1 milestone Mar 21, 2022
@m4rs-mt m4rs-mt added the bug label Mar 21, 2022
@m4rs-mt m4rs-mt modified the milestones: v1.1, v1.2 Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants