Skip to content

Commit

Permalink
[spirv] Check parent class when setting contains-alias-component of "…
Browse files Browse the repository at this point in the history
…this" (microsoft#4425)

When we handle `Load/Store` method of ByteAddressBuffer, we first load
the ByteAddressBuffer object if it is a reference of an alias variable.
`this` object is a reference of an alias variable if a
ByteAddressBuffer is a member of the class. The existing code checks
members of the class to determine if it has ByteAddressBuffer or not,
but it does not check its parent. Since we add all members of a parent
to its child class, we have to check members of the parent as well.
This commit fixes the issue and now `loadIfAliasVarRef(..)` properly
loads the access chain to ByteAddressBuffer.

For example,
```
// HLSL:
struct Base {
  ByteAddressBuffer buffer;
};

struct Child : Base {
  float load(in uint offset) {
    return asfloat(buffer.Load(offset));
  }
};

// Before:
%param_this = OpFunctionParameter %_ptr_Function_Child
%base = OpAccessChain %_ptr_Function_Base %param_this %uint_0
%ptr = OpAccessChain %_ptr_Function__ptr_Uniform_type_ByteAddressBuffer %base %int_0

OpAccessChain %_ptr_Uniform_uint %ptr %uint_0 ; <-- Error!!

// After:
%param_this = OpFunctionParameter %_ptr_Function_Child
%base = OpAccessChain %_ptr_Function_Base %param_this %uint_0
%ptr = OpAccessChain %_ptr_Function__ptr_Uniform_type_ByteAddressBuffer %base %int_0
%buffer = OpLoad %_ptr_Uniform_type_ByteAddressBuffer %ptr

OpAccessChain %_ptr_Uniform_uint %buffer %uint_0
```
  • Loading branch information
jaebaek authored May 3, 2022
1 parent f00b3c2 commit 21773f1
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 0 deletions.
8 changes: 8 additions & 0 deletions tools/clang/lib/SPIRV/AstTypeProbe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,14 @@ bool isOrContainsAKindOfStructuredOrByteBuffer(QualType type) {
if (isOrContainsAKindOfStructuredOrByteBuffer(field->getType()))
return true;
}

if (const auto *cxxDecl = type->getAsCXXRecordDecl()) {
for (const auto &base : cxxDecl->bases()) {
if (isOrContainsAKindOfStructuredOrByteBuffer(base.getType())) {
return true;
}
}
}
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// RUN: %dxc -T ps_6_0 -E main -fvk-use-dx-layout -fspv-reflect -fspv-target-env=vulkan1.1

// We want to check that we correctly set "contains-alias-component" when the
// class has a parent with ByteAddressBuffer, which later affects the result of
// `loadIfAliasVarRef(..)` by letting it emit a load instruction of the pointer.
// As a result, it prevents users of `loadIfAliasVarRef(..)` from just
// referencing an access chain without a proper load.

struct Base {
ByteAddressBuffer buffer;
};

struct Child : Base {
float load(in uint offset) {
// CHECK: %param_this = OpFunctionParameter %_ptr_Function_Child
// CHECK: [[base:%\w+]] = OpAccessChain %_ptr_Function_Base %param_this %uint_0
// CHECK: [[ptrToBuffer:%\w+]] = OpAccessChain %_ptr_Function__ptr_Uniform_type_ByteAddressBuffer [[base]] %int_0

// This test case is mainly intended to confirm that we emit the following instruction
// CHECK: [[buffer:%\w+]] = OpLoad %_ptr_Uniform_type_ByteAddressBuffer [[ptrToBuffer]]

// CHECK: OpAccessChain %_ptr_Uniform_uint [[buffer]] %uint_0
return asfloat(buffer.Load(offset));
}
};

void main(out float target : SV_Target) {
Child foo;
target = foo.load(0);
}
3 changes: 3 additions & 0 deletions tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,9 @@ TEST_F(FileTest, InheritanceCallMethodOfBase) {
runFileTest("oo.inheritance.call.base.method.hlsl", Expect::Success,
/* runValidation */ false);
}
TEST_F(FileTest, InheritanceBaseWithByteAddressBuffer) {
runFileTest("oo.inheritance.base-with-byte-address-buffer.hlsl");
}
TEST_F(FileTest, InheritanceCallMethodWithSameBaseMethodName) {
runFileTest("oo.call.method.with.same.base.method.name.hlsl");
}
Expand Down

0 comments on commit 21773f1

Please sign in to comment.