Skip to content

Fix test shader vulkan_tensor_postprocessing.spv #26

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
perim wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix test shader vulkan_tensor_postprocessing.spv #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 44 additions & 15 deletions framework/spirv_simulator.cpp
View file
Open in desktop

@roman-berger-arm roman-berger-arm Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The glsl spec says that the result of such a clamp is undefined before std450 and poison after. So in my eyes the current behaviour is valid for all versions before std450. We can return something nonsensical to represent the poison nature of the result, but otherwise I'm not sure if this should be worked around.

Original file line number Diff line number Diff line change
Expand Up @@ -3873,6 +3873,14 @@ bool SPIRVSimulator::TrySetComputeBuiltinValueAndRange(uint32_t result_id, const
set_scalar_range(0, total ? total - 1 : 0);
return true;
}
case spv::BuiltIn::BuiltInNumWorkgroups:
if (result_type.kind == Type::Kind::Vector)
{
// Same value for every invocation of a given dispatch.
SetValue(result_id, make_uvec3(nx, ny, nz));
return true;
}
break;
default:
break;
}
Expand Down Expand Up @@ -6982,18 +6990,25 @@ void SPIRVSimulator::GLSLExtHandler(uint32_t type_id,

for (uint32_t i = 0; i < type.vector.elem_count; ++i)
{
Value elem_result = (double)std::clamp(std::get<double>(vec->elems[i]),
std::get<double>(min_vec->elems[i]),
std::get<double>(max_vec->elems[i]));
// GLSL defines clamp(x, min, max) = max(min, min(x, max)), which is well-defined
// even when min > max (unlike std::clamp, which asserts in debug builds).
const double v = std::get<double>(vec->elems[i]);
const double lo = std::get<double>(min_vec->elems[i]);
const double hi = std::get<double>(max_vec->elems[i]);
Value elem_result = std::max(lo, std::min(v, hi));
result_vec->elems.push_back(elem_result);
}

SetValue(result_id, result_vec);
}
else if (type.kind == Type::Kind::Float)
{
Value result =
(double)std::clamp(std::get<double>(operand), std::get<double>(min_val), std::get<double>(max_val));
// GLSL defines clamp(x, min, max) = max(min, min(x, max)), which is well-defined
// even when min > max (unlike std::clamp, which asserts in debug builds).
const double v = std::get<double>(operand);
const double lo = std::get<double>(min_val);
const double hi = std::get<double>(max_val);
Value result = std::max(lo, std::min(v, hi));
SetValue(result_id, result);
}
else
Expand Down Expand Up @@ -7028,18 +7043,25 @@ void SPIRVSimulator::GLSLExtHandler(uint32_t type_id,

for (uint32_t i = 0; i < type.vector.elem_count; ++i)
{
Value elem_result = (uint64_t)std::clamp(std::get<uint64_t>(vec->elems[i]),
std::get<uint64_t>(min_vec->elems[i]),
std::get<uint64_t>(max_vec->elems[i]));
// GLSL defines clamp(x, min, max) = max(min, min(x, max)), which is well-defined
// even when min > max (unlike std::clamp, which asserts in debug builds).
const uint64_t v = std::get<uint64_t>(vec->elems[i]);
const uint64_t lo = std::get<uint64_t>(min_vec->elems[i]);
const uint64_t hi = std::get<uint64_t>(max_vec->elems[i]);
Value elem_result = std::max(lo, std::min(v, hi));
result_vec->elems.push_back(elem_result);
}

SetValue(result_id, result_vec);
}
else if (type.kind == Type::Kind::Int)
{
Value result = (uint64_t)std::clamp(
std::get<uint64_t>(operand), std::get<uint64_t>(min_val), std::get<uint64_t>(max_val));
// GLSL defines clamp(x, min, max) = max(min, min(x, max)), which is well-defined
// even when min > max (unlike std::clamp, which asserts in debug builds).
const uint64_t v = std::get<uint64_t>(operand);
const uint64_t lo = std::get<uint64_t>(min_val);
const uint64_t hi = std::get<uint64_t>(max_val);
Value result = std::max(lo, std::min(v, hi));
SetValue(result_id, result);
}
else
Expand Down Expand Up @@ -7074,18 +7096,25 @@ void SPIRVSimulator::GLSLExtHandler(uint32_t type_id,

for (uint32_t i = 0; i < type.vector.elem_count; ++i)
{
Value elem_result = (int64_t)std::clamp(std::get<int64_t>(vec->elems[i]),
std::get<int64_t>(min_vec->elems[i]),
std::get<int64_t>(max_vec->elems[i]));
// GLSL defines clamp(x, min, max) = max(min, min(x, max)), which is well-defined
// even when min > max (unlike std::clamp, which asserts in debug builds).
const int64_t v = std::get<int64_t>(vec->elems[i]);
const int64_t lo = std::get<int64_t>(min_vec->elems[i]);
const int64_t hi = std::get<int64_t>(max_vec->elems[i]);
Value elem_result = std::max(lo, std::min(v, hi));
result_vec->elems.push_back(elem_result);
}

SetValue(result_id, result_vec);
}
else if (type.kind == Type::Kind::Int)
{
Value result = (int64_t)std::clamp(
std::get<int64_t>(operand), std::get<int64_t>(min_val), std::get<int64_t>(max_val));
// GLSL defines clamp(x, min, max) = max(min, min(x, max)), which is well-defined
// even when min > max (unlike std::clamp, which asserts in debug builds).
const int64_t v = std::get<int64_t>(operand);
const int64_t lo = std::get<int64_t>(min_val);
const int64_t hi = std::get<int64_t>(max_val);
Value result = std::max(lo, std::min(v, hi));
SetValue(result_id, result);
}
else
Expand Down
67 changes: 67 additions & 0 deletions test/misc_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,73 @@ TEST_F(PointerResolutionTests, UndecoratedFunctionMatrixUsesPackedColumnStride)
EXPECT_EQ(GetPointerOffsetForTest(pointer), 16u);
}

TEST_F(GLSLExtInstructionTests, SClampAllowsMinGreaterThanMax)
{
// Regression test: a shader can legally reach a clamp(x, min, max) state with min > max
// (eg clamping to [0, width - 1] where width is 0). GLSL defines the result as
// max(min, min(x, max)), so the simulator must not abort (std::clamp asserts on min > max).
constexpr uint32_t result_id = 500;
constexpr uint32_t operand_id = 501;
constexpr uint32_t min_id = 502;
constexpr uint32_t max_id = 503;

::SPIRVSimulator::Value operand = int64_t{0};
::SPIRVSimulator::Value min_val = int64_t{0};
::SPIRVSimulator::Value max_val = int64_t{-1};
::SPIRVSimulator::Value captured_result;

EXPECT_CALL(*this, GetTypeByTypeId(CommonTypes::i32)).WillOnce(ReturnRef(types_.at(CommonTypes::i32)));
EXPECT_CALL(*this, GetValue(operand_id)).WillOnce(ReturnRef(operand));
EXPECT_CALL(*this, GetValue(min_id)).WillOnce(ReturnRef(min_val));
EXPECT_CALL(*this, GetValue(max_id)).WillOnce(ReturnRef(max_val));
EXPECT_CALL(*this, SetValue(result_id, _, true)).WillOnce(SaveArg<1>(&captured_result));
EXPECT_CALL(*this, TransferFlags(result_id, TypedEq<uint32_t>(operand_id)));
EXPECT_CALL(*this, TransferFlags(result_id, TypedEq<uint32_t>(min_id)));
EXPECT_CALL(*this, TransferFlags(result_id, TypedEq<uint32_t>(max_id)));

const std::vector<uint32_t> operands{ operand_id, min_id, max_id };
ExecuteGLSLExtInstruction(CommonTypes::i32, result_id, 45, operands);

ASSERT_TRUE(std::holds_alternative<int64_t>(captured_result));
EXPECT_EQ(std::get<int64_t>(captured_result), 0);
}

TEST_F(GLSLExtInstructionTests, SClampVectorClampsPerElement)
{
// max(min, min(x, max)) per element, including a min > max component.
constexpr uint32_t result_id = 510;
constexpr uint32_t operand_id = 511;
constexpr uint32_t min_id = 512;
constexpr uint32_t max_id = 513;

::SPIRVSimulator::Value operand =
std::make_shared<::SPIRVSimulator::VectorV>(std::initializer_list<int64_t>{ 15, -5, 0 });
::SPIRVSimulator::Value min_val =
std::make_shared<::SPIRVSimulator::VectorV>(std::initializer_list<int64_t>{ 2, 2, 0 });
::SPIRVSimulator::Value max_val =
std::make_shared<::SPIRVSimulator::VectorV>(std::initializer_list<int64_t>{ 10, 10, -1 });
::SPIRVSimulator::Value captured_result;

EXPECT_CALL(*this, GetTypeByTypeId(CommonTypes::ivec3)).WillOnce(ReturnRef(types_.at(CommonTypes::ivec3)));
EXPECT_CALL(*this, GetValue(operand_id)).WillOnce(ReturnRef(operand));
EXPECT_CALL(*this, GetValue(min_id)).WillOnce(ReturnRef(min_val));
EXPECT_CALL(*this, GetValue(max_id)).WillOnce(ReturnRef(max_val));
EXPECT_CALL(*this, SetValue(result_id, _, true)).WillOnce(SaveArg<1>(&captured_result));
EXPECT_CALL(*this, TransferFlags(result_id, TypedEq<uint32_t>(operand_id)));
EXPECT_CALL(*this, TransferFlags(result_id, TypedEq<uint32_t>(min_id)));
EXPECT_CALL(*this, TransferFlags(result_id, TypedEq<uint32_t>(max_id)));

const std::vector<uint32_t> operands{ operand_id, min_id, max_id };
ExecuteGLSLExtInstruction(CommonTypes::ivec3, result_id, 45, operands);

ASSERT_TRUE(std::holds_alternative<std::shared_ptr<::SPIRVSimulator::VectorV>>(captured_result));
const auto& result = std::get<std::shared_ptr<::SPIRVSimulator::VectorV>>(captured_result);
ASSERT_EQ(result->elems.size(), 3u);
EXPECT_EQ(std::get<int64_t>(result->elems[0]), 10);
EXPECT_EQ(std::get<int64_t>(result->elems[1]), 2);
EXPECT_EQ(std::get<int64_t>(result->elems[2]), 0);
}

class MemoryBarrierTests : public SPIRVSimulatorMockBase, public ::testing::Test
{};

Expand Down