Use SMin/UMin for SPIR-V target about slang HOT 12 CLOSED

I couldn't figure this one out why it is causing a problem.

I will need to figure out how to debug Falcor unit tests locally.
When I attempted, falcore.sln failed to open with my VisualStudio 2022.

Since this is a low priority clean up task, I am filing an issue and moving on to a higher priority task for now.

from slang.

I noticed that the falcor code is calling min from a static const:

struct LeapedHaltonSampleGenerator : ISampleGenerator
{
    struct Padded
    {
        LeapedHaltonSampleGenerator internal;
        uint2 _pad;
    };

    static const uint kDimensionCount = LeapedHaltonSequence::kDimensionCount;
    static const uint kInstanceCount = min(4096, LeapedHaltonSequence::kInstanceCount); //
...

Is there a way we can reproduce this with a similar test case?

from slang.

I noticed that the falcor code is calling min from a static const:

struct LeapedHaltonSampleGenerator : ISampleGenerator
{
    struct Padded
    {
        LeapedHaltonSampleGenerator internal;
        uint2 _pad;
    };

    static const uint kDimensionCount = LeapedHaltonSequence::kDimensionCount;
    static const uint kInstanceCount = min(4096, LeapedHaltonSequence::kInstanceCount); //
...

Is there a way we can reproduce this with a similar test case?

Interesting... I will test it with slang-test.

from slang.

I am not sure if I did it correctly.
But when I tried, the result was folded and there was no SMin/UMin instruction emitted.
I had a code like this,

enum TEST : int
{
   kConst = 1997
};

static int JKWAK = min(1996, int(TEST::kConst));

The emitted SPIR-V was like this,

%int_1996 = OpConstant %int 1996
OpStore %JKWAK %int_1996

from slang.

What is kConst is uint? Have you tried -O0? But the high level bit here is that things are compiling just fine.

from slang.

When I tried uint, I found a bug that it is treated as int.
But I don't think this is related to problem I was having, because it still doesn't emit UMin/SMin.

enum TEST : uint
{
   kConst = (1 << 31)
};

static uint JKWAK = min(4096, uint(TEST::kConst));

The code above yields the following,

%uint_2147483648 = OpConstant %uint 2147483648
OpStore %JKWAK %uint_2147483648

When I tried "(1 << 30)", it worked correctly but not for "(1 << 31).

The option -O0 hasn't shown any differences but I will keep playing with it.

from slang.

I will create a new issue for the problem with min.

from slang.

This is not a problem with min, but with overload resolution.

We should make conversion of an int literal to some other int type within range bearing a smaller cost so we can pick up min(uint,uint) when calling min(4096, uint(x)).

from slang.

I was able to build FalcorTest.exe locally.
On my previous attempt, the testing was done with an old binary that I built a few month ago.

With the latest binaries, I can reproduce the issue only with Release build not Debug build.

from slang.

The problem here is if the user writes:

static const uint val = min(4096, uint(0xFFFFFFFF));

What should we resolve?
If we resolve to min(int,int), then val will be 0xFFFFFFFF.
If we resolve to min(uint,uint), then val will be 4096.

from slang.

In this particular case, we shouldn't be resolving into min(int,int) because we know that 0xFFFFFFFF is out of range of int, but 4096 is within the range of uint, so we should prefer converting 4096 to uint and call min(uint, uint).

from slang.

This issue is blocked by another issue, #4104

from slang.