Print warning when unsigned and signed integer types used for binary operators

[jkwak-work]

Problem description When we compare unsigned integer and signed integer, as an example, Slang currently casts unsigned integer to signed integer. But this is opposite to how other languages do such as C/C++, GLSL and HLSL.

In C, when unsigned integer is used with a signed integer, the signed integer is promoted to unsigned integer. C99 defines the rule as following, called "usual arithmetic conversion":

One type between T1 and T2 is an signed integer type S, the other type is an unsigned integer type U. Apply the following rules:

1. If the integer conversion rank of U is greater than or equal to the integer conversion rank of S, C is U.
2. Otherwise, if S can represent all of the values of U, C is S.
3. Otherwise, C is the unsigned integer type corresponding to S. https://en.cppreference.com/w/cpp/language/usual_arithmetic_conversions

We should print a warning when binary operators are used with different sign-ness of integer types. And we should also follow the "usual arithmetic conversion".

The reason why Slang converts from unsigned to sign is because it follows the "ConversionCost" rule. But we should apply a different rule for the binary operators. Following lines show the Conversion costs for the sign-ness cost.

        // Conversions that are lossless, but change "kind"
        kConversionCost_UnsignedToSignedPromotion = 200,

        // Conversion from signed->unsigned integer of same or greater size
        kConversionCost_SignedToUnsignedConversion = 300,

Note that the cost of "uint -> int" is lower than "int -> uint".

The binary operators subject to this issue are following:

• operator+
• operator-
• operator*
• operator/
• operator%
• operator<
• operator<=
• operator>
• operator>=

Goal We should print a warning when binary operators are used with different sign-ness of integer types. And we should also follow the "usual arithmetic conversion".

Repro step Following test case can generate the HLSL/GLSL source code for the operator<(int,uint) case.

//TEST:SIMPLE:-target hlsl -entry PSMain -stage pixel
//TEST:SIMPLE:-target glsl -entry PSMain -stage pixel

StructuredBuffer<int32_t> i32Input;
StructuredBuffer<uint32_t> u32Input;
RWStructuredBuffer<int> outputBuffer;

void PSMain()
{
    bool result = (i32Input[0] < u32Input[1]);
    outputBuffer[0] = int(result);
}

Slang generates HLSL as following,

StructuredBuffer<int > i32Input_0 : register(t0);
StructuredBuffer<uint > u32Input_0 : register(t1);
RWStructuredBuffer<int > outputBuffer_0 : register(u0);

void PSMain()
{
    outputBuffer_0[0U] = int(i32Input_0.Load(0U) < int(u32Input_0.Load(1U)));
}

In order to inspect the DXIL, you can copy the code above and put it in the Shader Playground When the cast to "int" is removed, DXIL looks different.

  // With cast to int,
  // outputBuffer_0[0U] = int(i32Input_0.Load(0U) < int(u32Input_0.Load(1U)));
  %8 = icmp slt i32 %5, %7

  // Without cast to int,
  // outputBuffer_0[0U] = int(i32Input_0.Load(0U) < u32Input_0.Load(1U));
  %8 = icmp ult i32 %5, %7

Similarly, Slang generates GLSL as following,

#version 450
layout(row_major) uniform;
layout(row_major) buffer;

layout(std430, binding = 0) readonly buffer StructuredBuffer_int_t_0 {
    int _data[];
} i32Input_0;

layout(std430, binding = 1) readonly buffer StructuredBuffer_uint_t_0 {
    uint _data[];
} u32Input_0;

layout(std430, binding = 2) buffer StructuredBuffer_int_t_1 {
    int _data[];
} outputBuffer_0;

void main()
{
    outputBuffer_0._data[0U] = int(i32Input_0._data[0U] < int(u32Input_0._data[1U]));
}

When the cast to "int" is removed, SPIR-V asm looks different.

  // With cast to int,
  // outputBuffer_0._data[0U] = int(i32Input_0._data[0U] < int(u32Input_0._data[1U]));
         %27 = OpLoad %uint %26
         %28 = OpBitcast %int %27
         %30 = OpSLessThan %bool %18 %28

  // Without cast to int
  // outputBuffer_0._data[0U] = int(i32Input_0._data[0U] < u32Input_0._data[1U]);
         %28 = OpLoad %uint %27
         %30 = OpULessThan %bool %20 %28

Note I suspected that the ConversionCost might be set incorrectly. But it doesn't seem that is the case.

When there are overloading with "int" and 'uint", DXC prefers "int" over "uint". The following example shows the different DXIL between two cases where the first case defines only "USE_INT" and another case defines only "USE_UINT". When both "USE_INT" and "USE_UINT" are defined, the resulting DXIL was same to the case that defined only "USE_INT".

StructuredBuffer<int > i32Input : register(t0);
StructuredBuffer<uint32_t > u32Input : register(t1);
RWStructuredBuffer<int > outputBuffer : register(u0);

#define USE_INT
#define USE_UINT

#ifdef USE_INT
bool testOverload(int lhs, int rhs)
{
    return lhs < rhs;
}
#endif

#ifdef USE_UINT
bool testOverload(uint32_t lhs, uint32_t rhs)
{
    return lhs < rhs;
}
#endif

void PSMain()
{
    outputBuffer[0U] = int(testOverload(i32Input.Load(0U), u32Input.Load(1U)));
    return;
}

[bmillsNV]

Not needed for Q2, moving to Q3. @jkwak-work can you also estimate this one?