gpuweb / cts

WebGPU Conformance Test Suite
https://gpuweb.github.io/cts/
BSD 3-Clause "New" or "Revised" License
129 stars 82 forks source link

wgsl: Add shader regression tests for countOneBits and reverseBits for HLSL #1458

Open amaiorano opened 2 years ago

amaiorano commented 2 years ago

See this Tint bug. Basically, in HLSL, countbits and reversebits only accept and return uint - there are no int overloads. As a result, when the result of one of these calls was passed to a function that does have overloads, it would always call the uint overload.

For example, in WGSL, min(countOneBits(0), -1) should return -1, but was returning 0 because countOneBits (HLSL countbits)'s return type would be u32.

Similarly, min(reverseBits(-1), 0) should return -1, but was also returning 0 because reverseBits (HLSL reversebits)'s return type would be u32.

We need a couple of regression unit tests for these two cases. I quickly added two tests in countOneBits.spec.ts and reverseBits.spec.ts, but these are in the wrong place, and probably not how we want to write them. However, here they are as a guide:

  g.test('i32_min')
  .params(u =>
    u
      .combine('storageClass', ['uniform'] as const)
  )
  .fn(async t => {
    const cfg: Config = t.params;
    const min_of_builtin = (name: string, arg2: number): ExpressionBuilder => {
      return values => `min(${name}(${values.join(', ')}), ${arg2})`;
    };
    run(t, min_of_builtin('reverseBits', 0), [TypeI32], TypeI32, cfg, [
      { input: i32(-1), expected: i32(-1) }, // min(reverseBits(-1), 0) == -1
    ]);
  });

and

  g.test('i32_min')
  .params(u =>
    u
      .combine('storageClass', ['uniform'] as const)
  )
  .fn(async t => {
    const cfg: Config = t.params;
    const min_of_builtin = (name: string, arg2: number): ExpressionBuilder => {
      return values => `min(${name}(${values.join(', ')}), ${arg2})`;
    };

    run(t, min_of_builtin('countOneBits', -1), [TypeI32], TypeI32, cfg, [
      { input: i32(0), expected: i32(-1) }, // min(countOneBits(0), -1) == -1
    ]);
  });
amaiorano commented 2 years ago

This CL fixes this bug: https://dawn-review.googlesource.com/c/dawn/+/91001