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.

Sign up for GitHub

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

Check whether array element is fully specialized #6000

Merged
merged 5 commits into from
Jan 8, 2025
Merged

Check whether array element is fully specialized #6000

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f45cd07
Check whether array element is fully specialized
Jan 3, 2025
fac1fa5
add test
kaizhangNV Jan 3, 2025
4c70c8e
add all wrapper types into the check
kaizhangNV Jan 7, 2025
0431e56
add utility function to check if the type is wrapper type
kaizhangNV Jan 7, 2025
d68304a
Merge branch 'master' into spec_array
csyonghe Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions source/slang/slang-ir-specialize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ struct SpecializationContext
}
return false;
}
case kIROp_ArrayType:
{
auto array = as<IRArrayType>(inst);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this case is necessary.
If we have an type that is T.Differential[N], then it shall be encoded as IRArrayType(IRLookupWitness(w, "Differential"), N), and since IRLookupWitness has operand w, it won't appear in global scope, and therefore the IRArrayType itself won't appear directly in the global scope, and therefore it won't be reported as true by the default case logic.

Basically, if the element type is not fully specialized, then the array type itself shouldn't appear in the global scope. What IR are you seeing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this case this necessary, the unit test can reproduce the case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ask this question because in theory this shouldn’t happen, otherwise we will have the same issue for Ptr type and all other kinds of wrapper types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you dump the IR from the unit test, you will find out:

Ptr(specialize(%84, specialize(%150, Float, 1 : Int, %168)))	= get_field_addr(%callx5Fdata, %result)

this could result as:

	struct %GradInBuffer	: Type
	{
		field(%gradx5Fin, specialize(%82, lookupWitness( witness_table_t(%IDifferentiable)(Array(Float, 1)),   %37)))
	}

and finally this lookup table will give us

Array(lookupWitness(%168, %37), %N6)

here is the case I need to handle.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. In this case we do need to also handle all other opcodes by recursively checking their operands. Because we could have PtrType(lookupWitness()), or ArrayType(int, lookupwitness()), etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always hit this:

type argument 'float[1]' does not conform to the required interface 'IFoo'

not sure what's wrong regarding extending Array to conform to IFoo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your syntax you specified T:IFoo, since float !: IFoo, Array<float,1> !: IFoo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see the problem, I forgot removing that line.

OK, I will see if I can craft a shader to reproduce this issue.

Copy link
Collaborator

@csyonghe csyonghe Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a repro, you probably need to define an associatedtype in the interface for lookupwitness to occur inside an array type

for example, something like:

interface IFoo
{
      associatedtype FA : IFoo;
     int get();
}
struct X : IFoo { typealias FA = X;  int get() { return -1; } }
extension<T, int n> Array<T, n> : IFoo
{
      typealias FA = X;
}

int test<T:IFoo>(T.FA v[2]) {
     return v[0].get();
}

...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote another shader a little bit different from yours, but now I can get stable reproduce.

auto elementType = array->getElementType();
return isInstFullySpecialized(elementType);
}
}

// The default case is that a global value is always specialized.
Expand Down
43 changes: 43 additions & 0 deletions tests/bugs/gh-5776.slang
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute -shaderobj -output-using-type
//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 -profile sm_6_0 -use-dxil -output-using-type
//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cuda -shaderobj -output-using-type
//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cpu -shaderobj -output-using-type
//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -wgpu -output-using-type
//
struct WrappedBuffer<T : IDifferentiable>
{
StructuredBuffer<T> buffer;
int shape;

T get(int idx) { return buffer[idx]; }
}

struct GradInBuffer<T : IDifferentiable>
{
WrappedBuffer<T.Differential> grad_in;
}

struct CallData
{
WrappedBuffer<float[1]> grad_in;
}


//TEST_INPUT: set call_data.grad_in.buffer = ubuffer(data=[1.0 2.0], stride=4);
ParameterBlock<CallData> call_data;


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


[shader("compute")]
[numthreads(1, 1, 1)]
void computeMain()
{

float[1] data1 = call_data.grad_in.buffer[0];
float[1] data2 = call_data.grad_in.get(1);
outputBuffer[0] = data1[0];
outputBuffer[1] = data2[0];
}
3 changes: 3 additions & 0 deletions tests/bugs/gh-5776.slang.expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type: float
1.000000
2.000000
Loading