Skip to content

spirv-val: Add OpSizeOf #6497

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

Open
spencer-lunarg wants to merge 3 commits into
base: main
Choose a base branch
from
+213 −10
Open

spirv-val: Add OpSizeOf #6497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47a3a1f
spirv-val: Add OpSizeOf
spencer-lunarg Jan 17, 2026
9633b76
spirv-val: Add Concrete pointer check
spencer-lunarg Jan 18, 2026
c3e0585
Ban untyped pointers in OpSizeOf
spencer-lunarg Jan 25, 2026
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
26 changes: 26 additions & 0 deletions source/val/validate_misc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,27 @@ spv_result_t ValidateShaderClock(ValidationState_t& _,
return SPV_SUCCESS;
}

spv_result_t ValidateSizeOf(ValidationState_t& _, const Instruction* inst) {
Copy link
Copy Markdown
Contributor

@alan-baker alan-baker Jan 17, 2026

Choose a reason for hiding this comment

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

For completeness, could you add check for the pointer operand? It must point to a concrete. I don't think untyped pointers are allowed here.

Copy link
Copy Markdown
Contributor Author

@spencer-lunarg spencer-lunarg Jan 18, 2026

Choose a reason for hiding this comment

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

so I tried to add a IsConcreteType helper, from what I can read from SPV_KHR_untyped_pointers, there is no reason I can see why an untyped pointer would not be allowed, as long as it is Physical

Copy link
Copy Markdown
Contributor

@alan-baker alan-baker Jan 19, 2026

Choose a reason for hiding this comment

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

An untyped pointer could be returned I suppose, but I meant pointer that OpSizeOf has an operand which is a pointer and it returns the size of the object pointed to by that pointer. Untyped pointers don't carry the information to make that check easy. Also, I don't think the extension modified this instruction. So you could have a function variable containing an untyped pointer (say to cross-workgroup), but not an untyped function variable I think.

Copy link
Copy Markdown
Contributor Author

@spencer-lunarg spencer-lunarg Jan 19, 2026

Choose a reason for hiding this comment

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

ok, so ya, this makes sense... I guess my question is I am over following the spec, trying to think where in the spec would be where to make a change to make this explicit

For this, since I did the check as IsConcreteType, where would I then look for the untyped pointer exception... is this suppose to mean that an untyped pointer is abstract?

Copy link
Copy Markdown
Contributor

@alan-baker alan-baker Jan 19, 2026

Choose a reason for hiding this comment

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

I think it's like the following: if pointer has OpTypePointer, then check that the pointee is concrete. Because OpenCL uses Physical32 or Physical64 addressing model, all pointers (including untyped) are physical, and thus concrete. I'm on the fence about banning untyped pointers as the pointer operand since OpSizeOf is not particularly well described. I briefly spent some time on godbolt.org trying to generate it and I couldn't find a situation where it was not statically known very easily.

Copy link
Copy Markdown
Contributor Author

@spencer-lunarg spencer-lunarg Jan 25, 2026

Choose a reason for hiding this comment

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

updated with a condition to ban untyped

const uint32_t result_type = inst->type_id();
if (!_.IsIntScalarType(result_type, 32)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Expected OpSizeOf Result Type to be a 32-bit int scalar.";
}

uint32_t pointer_id = inst->GetOperandAs<uint32_t>(2);
if (!_.IsConcreteType(pointer_id)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "OpSizeOf Pointer operand is not concrete.";
} else if (_.FindDef(pointer_id)->opcode() ==
spv::Op::OpTypeUntypedPointerKHR) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "OpSizeOf Pointer operand is to an untyped pointer, which size "
"is not well defined.";
}

return SPV_SUCCESS;
}

spv_result_t ValidateAssumeTrue(ValidationState_t& _, const Instruction* inst) {
const auto operand_type_id = _.GetOperandTypeId(inst, 0);
if (!operand_type_id || !_.IsBoolScalarType(operand_type_id)) {
Expand Down Expand Up @@ -193,6 +214,11 @@ spv_result_t MiscPass(ValidationState_t& _, const Instruction* inst) {
return error;
}
break;
case spv::Op::OpSizeOf:
if (auto error = ValidateSizeOf(_, inst)) {
return error;
}
break;
case spv::Op::OpAssumeTrueKHR:
if (auto error = ValidateAssumeTrue(_, inst)) {
return error;
Expand Down
66 changes: 56 additions & 10 deletions source/val/validation_state.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "source/val/construct.h"
#include "source/val/function.h"
#include "spirv-tools/libspirv.h"
#include "spirv/unified1/spirv.hpp11"

namespace spvtools {
namespace val {
Expand Down Expand Up @@ -927,22 +928,16 @@ uint32_t ValidationState_t::GetComponentType(uint32_t id) const {

case spv::Op::OpTypeArray:
case spv::Op::OpTypeRuntimeArray:
return inst->word(2);

case spv::Op::OpTypeVector:
return inst->word(2);

case spv::Op::OpTypeMatrix:
return GetComponentType(inst->word(2));

case spv::Op::OpTypeVectorIdEXT:
case spv::Op::OpTypeCooperativeMatrixNV:
case spv::Op::OpTypeCooperativeMatrixKHR:
case spv::Op::OpTypeVectorIdEXT:
return inst->word(2);

case spv::Op::OpTypeTensorARM:
return inst->word(2);

case spv::Op::OpTypeMatrix:
return GetComponentType(inst->word(2));

default:
break;
}
Expand Down Expand Up @@ -1632,6 +1627,57 @@ bool ValidationState_t::IsDescriptorHeapBaseVariable(const Instruction* inst) {
is_heap_base);
}

// From the spec (SPIRV.html#PhysicalPointerType)
bool ValidationState_t::IsPhysicalPointerType(uint32_t id) const {
const Instruction* inst = FindDef(id);
const spv::Op opcode = inst->opcode();
if (opcode != spv::Op::OpTypePointer &&
opcode != spv::Op::OpTypeUntypedPointerKHR) {
return false;
}

const spv::AddressingModel am = addressing_model();
if (am == spv::AddressingModel::Logical) {
return false;
} else if (am == spv::AddressingModel::Physical32 ||
am == spv::AddressingModel::Physical64) {
return true;
} else if (am == spv::AddressingModel::PhysicalStorageBuffer64) {
const spv::StorageClass storage_class = spv::StorageClass(inst->word(2));
return storage_class == spv::StorageClass::PhysicalStorageBuffer;
}

assert(0);
return false;
}

// From the spec (SPIRV.html#Numerical)
bool ValidationState_t::IsNumericalType(uint32_t id) const {
const Instruction* inst = FindDef(id);
const spv::Op opcode = inst->opcode();
return opcode == spv::Op::OpTypeInt || opcode == spv::Op::OpTypeFloat;
}

// From the spec (SPIRV.html#Concrete)
bool ValidationState_t::IsConcreteType(uint32_t id) const {
const Instruction* inst = FindDef(id);
const spv::Op opcode = inst->opcode();

if (opcode == spv::Op::OpTypeStruct) {
// all elements must be concrete
for (uint32_t i = 1; i < inst->operands().size(); ++i) {
if (!IsConcreteType(inst->GetOperandAs<uint32_t>(i))) {
return false;
}
}
return true;
}

const uint32_t component_type = GetComponentType(id);
return IsNumericalType(component_type) ||
IsPhysicalPointerType(component_type);
}

spv_result_t ValidationState_t::CooperativeMatrixShapesMatch(
const Instruction* inst, uint32_t result_type_id, uint32_t m2,
bool is_conversion, bool swap_row_col) {
Expand Down
3 changes: 3 additions & 0 deletions source/val/validation_state.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,9 @@ class ValidationState_t {
bool IsTensorType(uint32_t id) const;
bool IsDescriptorType(spv::Op opcode) const;
bool IsDescriptorType(uint32_t id) const;
bool IsPhysicalPointerType(uint32_t id) const;
bool IsNumericalType(uint32_t id) const;
bool IsConcreteType(uint32_t id) const;
// When |length| is not 0, return true only if the array length is equal to
// |length| and the array length is not defined by a specialization constant.
bool IsArrayType(uint32_t id, uint64_t length = 0) const;
Expand Down
128 changes: 128 additions & 0 deletions test/val/val_misc_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,134 @@ TEST_F(ValidateMisc, SizeOfValid) {
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
}

TEST_F(ValidateMisc, SizeOfStructValid) {
const std::string spirv = R"(
OpCapability Addresses
OpCapability Kernel
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %f "f"
%void = OpTypeVoid
%i32 = OpTypeInt 32 0
%ptr = OpTypePointer CrossWorkgroup %i32
%struct_a = OpTypeStruct %ptr %i32
%struct_b = OpTypeStruct %struct_a %ptr
%fnTy = OpTypeFunction %void
%f = OpFunction %void None %fnTy
%entry = OpLabel
%s = OpSizeOf %i32 %struct_b
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
}

TEST_F(ValidateMisc, SizeOfStructWithAbstract) {
const std::string spirv = R"(
OpCapability Addresses
OpCapability Kernel
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %f "f"
%void = OpTypeVoid
%i32 = OpTypeInt 32 0
%bool = OpTypeBool
%ptr = OpTypePointer CrossWorkgroup %i32
%struct_a = OpTypeStruct %ptr %bool
%struct_b = OpTypeStruct %struct_a %ptr
%fnTy = OpTypeFunction %void
%f = OpFunction %void None %fnTy
%entry = OpLabel
%s = OpSizeOf %i32 %struct_b
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
EXPECT_EQ(SPV_ERROR_INVALID_DATA,
ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("OpSizeOf Pointer operand is not concrete"));
}

TEST_F(ValidateMisc, SizeOfUntyped) {
const std::string spirv = R"(
OpCapability Addresses
OpCapability UntypedPointersKHR
OpCapability Kernel
OpExtension "SPV_KHR_untyped_pointers"
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %f "f"
%void = OpTypeVoid
%i32 = OpTypeInt 32 0
%fnTy = OpTypeFunction %void
%untyped_ptr = OpTypeUntypedPointerKHR CrossWorkgroup
%f = OpFunction %void None %fnTy
%entry = OpLabel
%s = OpSizeOf %i32 %untyped_ptr
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
EXPECT_EQ(SPV_ERROR_INVALID_DATA,
ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("OpSizeOf Pointer operand is to an untyped pointer, "
"which size is not well defined"));
}

TEST_F(ValidateMisc, SizeOfFloat) {
const std::string spirv = R"(
OpCapability Addresses
OpCapability Kernel
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %f "f"
%void = OpTypeVoid
%f32 = OpTypeFloat 32
%ptr = OpTypePointer CrossWorkgroup %f32
%fnTy = OpTypeFunction %void
%f = OpFunction %void None %fnTy
%entry = OpLabel
%s = OpSizeOf %f32 %ptr
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
EXPECT_EQ(SPV_ERROR_INVALID_DATA,
ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("Expected OpSizeOf Result Type to be a 32-bit int scalar"));
}

TEST_F(ValidateMisc, SizeOfVector) {
const std::string spirv = R"(
OpCapability Addresses
OpCapability Kernel
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %f "f"
%void = OpTypeVoid
%i32 = OpTypeInt 32 0
%v2i32 = OpTypeVector %i32 2
%ptr = OpTypePointer CrossWorkgroup %v2i32
%fnTy = OpTypeFunction %void
%f = OpFunction %void None %fnTy
%entry = OpLabel
%s = OpSizeOf %v2i32 %ptr
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
EXPECT_EQ(SPV_ERROR_INVALID_DATA,
ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("Expected OpSizeOf Result Type to be a 32-bit int scalar"));
}

const std::string ShaderClockSpirv = R"(
OpCapability Shader
OpCapability Int64
Expand Down