Skip to content

Commit

Permalink
Adding some CTS variants for indirect command buffers. (#17846)
Browse files Browse the repository at this point in the history 
This is a simple start that tests a few commands to test the
infrastructure for record/replay of indirect command buffers and the
validation mechanism. We don't have a good way in the CTS yet to
conditionally run tests based on whether validation is compiled into the
build but I'll be looking into that for testing failure cases in future
PRs (for now I've just tested manually to verify errors are propagated).
  • Loading branch information
@benvanik
benvanik committed Jul 11, 2024
1 parent c1611cd commit 85e0da6
Show file tree
Hide file tree
Showing 28 changed files with 1,385 additions and 958 deletions.
5 changes: 5 additions & 0 deletions runtime/src/iree/hal/command_buffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,11 @@ iree_hal_buffer_binding_table_empty(void) {
return table;
}

static inline bool iree_hal_buffer_binding_table_is_empty(
iree_hal_buffer_binding_table_t binding_table) {
return binding_table.count == 0;
}

// Returns an unretained buffer specified in |buffer_ref| or from
// |binding_table| with the slot specified if indirect. If the caller needs to
// preserve the buffer for longer than the (known) lifetime of the binding table
Expand Down
52 changes: 31 additions & 21 deletions runtime/src/iree/hal/command_buffer_validation.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,16 @@ static iree_status_t iree_hal_command_buffer_validate_binding_requirements(
// it are in range.
if (requirements.max_byte_offset > 0) {
iree_device_size_t end = binding.offset + requirements.max_byte_offset;
if (IREE_UNLIKELY(end > binding.length)) {
return iree_make_status(IREE_STATUS_OUT_OF_RANGE,
"at least one command attempted to access an "
"address outside of the valid bound buffer "
"range (length=%" PRIdsz ", end(inc)=%" PRIdsz
", binding offset=%" PRIdsz
", binding length=%" PRIdsz ")",
requirements.max_byte_offset, end - 1,
binding.offset, binding.length);
if (IREE_UNLIKELY(end > binding.offset + binding.length)) {
return iree_make_status(
IREE_STATUS_OUT_OF_RANGE,
"at least one command attempted to access an "
"address outside of the valid bound buffer "
"range (length=%" PRIdsz ", end(inc)=%" PRIdsz
", binding offset=%" PRIdsz ", binding length=%" PRIdsz
", binding end(inc)=%" PRIdsz ")",
requirements.max_byte_offset, end - 1, binding.offset, binding.length,
binding.offset + binding.length - 1);
}
}

Expand Down Expand Up @@ -188,8 +189,11 @@ static iree_status_t iree_hal_command_buffer_validate_buffer_requirements(
table_requirements->type |= requirements.type;
table_requirements->max_byte_offset = iree_max(
table_requirements->max_byte_offset, requirements.max_byte_offset);
table_requirements->min_byte_alignment = iree_device_size_lcm(
table_requirements->min_byte_alignment, requirements.min_byte_alignment);
if (requirements.min_byte_alignment) {
table_requirements->min_byte_alignment =
iree_device_size_lcm(table_requirements->min_byte_alignment,
requirements.min_byte_alignment);
}

return iree_ok_status();
}
Expand Down Expand Up @@ -430,14 +434,19 @@ iree_status_t iree_hal_command_buffer_copy_buffer_validation(
// Check for overlap - just like memcpy we don't handle that.
// Note that it's only undefined behavior if violated so we are ok if tricky
// situations (subspans of subspans of binding table subranges etc) make it
// through.
if (iree_hal_buffer_test_overlap(source_ref.buffer, source_ref.offset,
source_ref.length, target_ref.buffer,
target_ref.offset, target_ref.length) !=
IREE_HAL_BUFFER_OVERLAP_DISJOINT) {
return iree_make_status(
IREE_STATUS_INVALID_ARGUMENT,
"source and target ranges overlap within the same buffer");
// through. This is only possible if both buffers are directly referenced -
// we _could_ try to catch this for indirect references by stashing the
// overlap check metadata for validation when the binding table is available
// but that's too costly to be worth it.
if (source_ref.buffer && target_ref.buffer) {
if (iree_hal_buffer_test_overlap(source_ref.buffer, source_ref.offset,
source_ref.length, target_ref.buffer,
target_ref.offset, target_ref.length) !=
IREE_HAL_BUFFER_OVERLAP_DISJOINT) {
return iree_make_status(
IREE_STATUS_INVALID_ARGUMENT,
"source and target ranges overlap within the same buffer");
}
}

return iree_ok_status();
Expand Down Expand Up @@ -571,10 +580,11 @@ iree_status_t iree_hal_command_buffer_push_descriptor_set_validation(
// TODO(benvanik): validate set index.

// TODO(benvanik): use pipeline layout to derive usage and access bits.
// For now we conservatively say _any_ access may be performed (read/write).
iree_hal_buffer_binding_requirements_t requirements = {
.required_compatibility = IREE_HAL_BUFFER_COMPATIBILITY_QUEUE_DISPATCH,
// .usage = IREE_HAL_BUFFER_USAGE_DISPATCH_...,
// .access = IREE_HAL_MEMORY_ACCESS_...,
.usage = IREE_HAL_BUFFER_USAGE_DISPATCH_STORAGE,
.access = IREE_HAL_MEMORY_ACCESS_ANY,
.type = IREE_HAL_MEMORY_TYPE_DEVICE_VISIBLE,
};
for (iree_host_size_t i = 0; i < binding_count; ++i) {
Expand Down
42 changes: 42 additions & 0 deletions runtime/src/iree/hal/cts/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ set(IREE_ALL_CTS_TESTS
"allocator"
"buffer_mapping"
"command_buffer"
"command_buffer_copy_buffer"
"command_buffer_dispatch"
"command_buffer_fill_buffer"
"command_buffer_push_constants"
"command_buffer_update_buffer"
"descriptor_set_layout"
"driver"
"event"
Expand Down Expand Up @@ -91,6 +94,19 @@ iree_cc_library(
TESTONLY
)

iree_cc_library(
NAME
command_buffer_copy_buffer_test_library
HDRS
"command_buffer_copy_buffer_test.h"
DEPS
::cts_test_base
iree::base
iree::hal
iree::testing::gtest
TESTONLY
)

iree_cc_library(
NAME
command_buffer_dispatch_test_library
Expand All @@ -104,6 +120,19 @@ iree_cc_library(
TESTONLY
)

iree_cc_library(
NAME
command_buffer_fill_buffer_test_library
HDRS
"command_buffer_fill_buffer_test.h"
DEPS
::cts_test_base
iree::base
iree::hal
iree::testing::gtest
TESTONLY
)

iree_cc_library(
NAME
command_buffer_push_constants_test_library
Expand All @@ -117,6 +146,19 @@ iree_cc_library(
TESTONLY
)

iree_cc_library(
NAME
command_buffer_update_buffer_test_library
HDRS
"command_buffer_update_buffer_test.h"
DEPS
::cts_test_base
iree::base
iree::hal
iree::testing::gtest
TESTONLY
)

iree_cc_library(
NAME
descriptor_set_layout_test_library
Expand Down
18 changes: 6 additions & 12 deletions runtime/src/iree/hal/cts/allocator_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,21 @@
#include "iree/testing/gtest.h"
#include "iree/testing/status_matchers.h"

namespace iree {
namespace hal {
namespace cts {
namespace iree::hal::cts {

namespace {

constexpr iree_device_size_t kAllocationSize = 1024;

} // namespace

class allocator_test : public CTSTestBase<> {};
class AllocatorTest : public CTSTestBase<> {};

// All allocators must support some baseline capabilities.
//
// Certain capabilities or configurations are optional and may vary between
// driver implementations or target devices, such as:
// IREE_HAL_MEMORY_TYPE_HOST_LOCAL | IREE_HAL_MEMORY_TYPE_DEVICE_LOCAL
// IREE_HAL_BUFFER_USAGE_MAPPING
TEST_F(allocator_test, BaselineBufferCompatibility) {
TEST_F(AllocatorTest, BaselineBufferCompatibility) {
// Need at least one way to get data between the host and device.
iree_hal_buffer_params_t host_local_params = {0};
host_local_params.type =
Expand Down Expand Up @@ -80,7 +76,7 @@ TEST_F(allocator_test, BaselineBufferCompatibility) {
IREE_HAL_BUFFER_COMPATIBILITY_QUEUE_DISPATCH));
}

TEST_F(allocator_test, AllocateBuffer) {
TEST_F(AllocatorTest, AllocateBuffer) {
iree_hal_buffer_params_t params = {0};
params.type = IREE_HAL_MEMORY_TYPE_DEVICE_LOCAL;
params.usage = IREE_HAL_BUFFER_USAGE_TRANSFER;
Expand All @@ -102,7 +98,7 @@ TEST_F(allocator_test, AllocateBuffer) {

// While empty allocations aren't particularly useful, they can occur in
// practice so we should at least be able to create them without errors.
TEST_F(allocator_test, AllocateEmptyBuffer) {
TEST_F(AllocatorTest, AllocateEmptyBuffer) {
iree_hal_buffer_params_t params = {0};
params.type = IREE_HAL_MEMORY_TYPE_DEVICE_LOCAL;
params.usage = IREE_HAL_BUFFER_USAGE_TRANSFER;
Expand All @@ -113,8 +109,6 @@ TEST_F(allocator_test, AllocateEmptyBuffer) {
iree_hal_buffer_release(buffer);
}

} // namespace cts
} // namespace hal
} // namespace iree
} // namespace iree::hal::cts

#endif // IREE_HAL_CTS_ALLOCATOR_TEST_H_
42 changes: 19 additions & 23 deletions runtime/src/iree/hal/cts/buffer_mapping_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
#include "iree/testing/gtest.h"
#include "iree/testing/status_matchers.h"

namespace iree {
namespace hal {
namespace cts {
namespace iree::hal::cts {

using ::testing::ContainerEq;

Expand All @@ -42,7 +40,7 @@ constexpr iree_device_size_t kDefaultAllocationSize = 1024;
// * write with an offset and length
// * write into a subspan of a buffer

class buffer_mapping_test : public CTSTestBase<> {
class BufferMappingTest : public CTSTestBase<> {
protected:
void AllocateUninitializedBuffer(iree_device_size_t buffer_size,
iree_hal_buffer_t** out_buffer) {
Expand All @@ -59,7 +57,7 @@ class buffer_mapping_test : public CTSTestBase<> {
}
};

TEST_F(buffer_mapping_test, AllocatorSupportsBufferMapping) {
TEST_F(BufferMappingTest, AllocatorSupportsBufferMapping) {
iree_hal_buffer_params_t params = {0};
params.type = IREE_HAL_MEMORY_TYPE_HOST_VISIBLE;
params.usage = IREE_HAL_BUFFER_USAGE_MAPPING;
Expand All @@ -83,7 +81,7 @@ TEST_F(buffer_mapping_test, AllocatorSupportsBufferMapping) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, ZeroWholeBuffer) {
TEST_F(BufferMappingTest, ZeroWholeBuffer) {
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(kDefaultAllocationSize, &buffer);

Expand All @@ -102,7 +100,7 @@ TEST_F(buffer_mapping_test, ZeroWholeBuffer) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, ZeroWithOffset) {
TEST_F(BufferMappingTest, ZeroWithOffset) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand All @@ -128,7 +126,7 @@ TEST_F(buffer_mapping_test, ZeroWithOffset) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, ZeroSubspan) {
TEST_F(BufferMappingTest, ZeroSubspan) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -171,7 +169,7 @@ TEST_F(buffer_mapping_test, ZeroSubspan) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, FillEmpty) {
TEST_F(BufferMappingTest, FillEmpty) {
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(kDefaultAllocationSize, &buffer);

Expand All @@ -195,7 +193,7 @@ TEST_F(buffer_mapping_test, FillEmpty) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, FillWholeBuffer) {
TEST_F(BufferMappingTest, FillWholeBuffer) {
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(kDefaultAllocationSize, &buffer);

Expand All @@ -217,7 +215,7 @@ TEST_F(buffer_mapping_test, FillWholeBuffer) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, FillWithOffset) {
TEST_F(BufferMappingTest, FillWithOffset) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand All @@ -244,7 +242,7 @@ TEST_F(buffer_mapping_test, FillWithOffset) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, FillSubspan) {
TEST_F(BufferMappingTest, FillSubspan) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -288,7 +286,7 @@ TEST_F(buffer_mapping_test, FillSubspan) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, ReadData) {
TEST_F(BufferMappingTest, ReadData) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -325,7 +323,7 @@ TEST_F(buffer_mapping_test, ReadData) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, ReadDataSubspan) {
TEST_F(BufferMappingTest, ReadDataSubspan) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -368,7 +366,7 @@ TEST_F(buffer_mapping_test, ReadDataSubspan) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, WriteDataWholeBuffer) {
TEST_F(BufferMappingTest, WriteDataWholeBuffer) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand All @@ -390,7 +388,7 @@ TEST_F(buffer_mapping_test, WriteDataWholeBuffer) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, WriteDataWithOffset) {
TEST_F(BufferMappingTest, WriteDataWithOffset) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand All @@ -417,7 +415,7 @@ TEST_F(buffer_mapping_test, WriteDataWithOffset) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, WriteDataSubspan) {
TEST_F(BufferMappingTest, WriteDataSubspan) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -459,7 +457,7 @@ TEST_F(buffer_mapping_test, WriteDataSubspan) {
iree_hal_buffer_release(buffer);
}

TEST_F(buffer_mapping_test, CopyData) {
TEST_F(BufferMappingTest, CopyData) {
iree_hal_buffer_t* buffer_a = NULL;
iree_hal_buffer_t* buffer_b = NULL;
AllocateUninitializedBuffer(kDefaultAllocationSize, &buffer_a);
Expand Down Expand Up @@ -490,7 +488,7 @@ TEST_F(buffer_mapping_test, CopyData) {

// Maps a buffer range for reading from device -> host.
// This is roughly what iree_hal_buffer_map_read does internally.
TEST_F(buffer_mapping_test, MapRangeRead) {
TEST_F(BufferMappingTest, MapRangeRead) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -520,7 +518,7 @@ TEST_F(buffer_mapping_test, MapRangeRead) {

// Maps a buffer range for writing from host -> device.
// This is roughly what iree_hal_buffer_map_write does internally.
TEST_F(buffer_mapping_test, MapRangeWrite) {
TEST_F(BufferMappingTest, MapRangeWrite) {
iree_device_size_t buffer_size = 16;
iree_hal_buffer_t* buffer = NULL;
AllocateUninitializedBuffer(buffer_size, &buffer);
Expand Down Expand Up @@ -550,8 +548,6 @@ TEST_F(buffer_mapping_test, MapRangeWrite) {
iree_hal_buffer_release(buffer);
}

} // namespace cts
} // namespace hal
} // namespace iree
} // namespace iree::hal::cts

#endif // IREE_HAL_CTS_BUFFER_MAPPING_TEST_H_
Loading

0 comments on commit 85e0da6

Please sign in to comment.