Skip to content

Commit 2c88f57

Browse files
[Vulkan] Let tensor wrap an external buffer with caller-supplied sizes (pytorch#22031)
Follow-up to pytorch#21930, which added external-image storage. `vTensorStorage` had no buffer equivalent, so a tensor could only be built over externally owned storage if that storage was a `VulkanImage`. This adds an `external_buffer` parameter beside `external_image`, along with a `vTensorStorage` constructor and a `ComputeGraph::add_tensor` overload taking a `VulkanBuffer`. The buffer is held as a `VulkanBuffer` copy, which aliases the handle with `is_copy_` set, so the view never frees memory it does not own. At most one of the two may be set. ## Files - `backends/vulkan/runtime/api/containers/Tensor.h` — `external_buffer` on the `vTensor` constructor, and the `vTensorStorage` buffer constructor. - `backends/vulkan/runtime/api/containers/Tensor.cpp` — the buffer storage constructor, external-storage selection, and the size check. - `backends/vulkan/runtime/graph/ComputeGraph.h`, `.cpp` — an `add_tensor` overload taking sizes, dtype, memory layout and a buffer. - `backends/vulkan/test/vulkan_compute_api_test.cpp` — 3 new tests. ## Testing `vulkan_compute_api_test` New coverage: wrapping a buffer at `[1, 37, 8, 64]` and keeping the sizes and buffer storage type; rejecting sizes larger than the buffer; and a view reporting `is_copy_of` the source, with the source buffer still live after the view is destroyed and deferred cleanup has run. cc @SS-JIA @manuelcandales @digantdesai @cbilgin
1 parent b2fd947 commit 2c88f57

5 files changed

Lines changed: 158 additions & 9 deletions

File tree

‎backends/vulkan/runtime/api/containers/Tensor.cpp‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,23 @@ vTensorStorage::vTensorStorage(
732732
allocate_memory)),
733733
last_access_{} {}
734734

735+
namespace {
736+
737+
std::shared_ptr<vTensorStorage> acquire_external_storage(
738+
Context* const context,
739+
const vkapi::VulkanImage* image,
740+
const vkapi::VulkanBuffer* buffer,
741+
const vkapi::ScalarType dtype) {
742+
VK_CHECK_COND(
743+
image == nullptr || buffer == nullptr,
744+
"a tensor can wrap an image or a buffer, not both");
745+
return image != nullptr
746+
? std::make_shared<vTensorStorage>(context, *image)
747+
: std::make_shared<vTensorStorage>(context, *buffer, dtype);
748+
}
749+
750+
} // namespace
751+
735752
vTensorStorage::vTensorStorage(
736753
Context* const context,
737754
const vkapi::VulkanImage& image)
@@ -747,6 +764,20 @@ vTensorStorage::vTensorStorage(
747764
buffer_(vkapi::VulkanBuffer()),
748765
last_access_{} {}
749766

767+
vTensorStorage::vTensorStorage(
768+
Context* const context,
769+
const vkapi::VulkanBuffer& buffer,
770+
const vkapi::ScalarType dtype)
771+
: context_(context),
772+
storage_type_{utils::kBuffer},
773+
image_extents_({0, 0, 0}),
774+
buffer_length_{
775+
static_cast<int64_t>(buffer.mem_size() / vkapi::element_size(dtype))},
776+
buffer_offset_{0},
777+
image_(vkapi::VulkanImage()),
778+
buffer_(buffer),
779+
last_access_{} {}
780+
750781
vTensorStorage::~vTensorStorage() {
751782
flush();
752783
}
@@ -852,7 +883,8 @@ vTensor::vTensor(
852883
const utils::GPUMemoryLayout memory_layout,
853884
const bool allocate_memory,
854885
const utils::AxisMapLayout axis_map_layout,
855-
const vkapi::VulkanImage* external_image)
886+
const vkapi::VulkanImage* external_image,
887+
const vkapi::VulkanBuffer* external_buffer)
856888
: dtype_(get_effective_scalar_type(context, dtype, memory_layout)),
857889
packed_dim_info_(calculate_packed_dim_info(memory_layout, storage_type)),
858890
// Calculate tensor metadata
@@ -881,8 +913,12 @@ vTensor::vTensor(
881913
buffer_meta_(),
882914
// Construct Tensor storage
883915
storage_(
884-
external_image != nullptr
885-
? std::make_shared<vTensorStorage>(context, *external_image)
916+
(external_image != nullptr || external_buffer != nullptr)
917+
? acquire_external_storage(
918+
context,
919+
external_image,
920+
external_buffer,
921+
dtype_)
886922
: std::make_shared<vTensorStorage>(
887923
context,
888924
storage_type,
@@ -892,15 +928,18 @@ vTensor::vTensor(
892928
dtype_,
893929
physical_numel_,
894930
allocate_memory)) {
895-
// An external image was sized by its creator, so nothing guarantees it
896-
// matches the metadata derived from `sizes`. The extents feed the
897-
// shader-visible logical limits below, so they have to agree.
931+
// External storage was sized by its creator, so nothing guarantees it fits
932+
// the metadata derived from `sizes`. An image must match exactly, since its
933+
// extents feed the shader-visible logical limits below; a buffer need only
934+
// be large enough.
898935
if (external_image != nullptr) {
899936
VK_CHECK_COND(
900937
storage_->image_extents_ ==
901938
calculate_image_extents(
902939
dtype_, packed_dim_info_, padded_sizes_, axis_map_),
903940
"external image extents do not match the requested tensor sizes");
941+
} else if (external_buffer != nullptr) {
942+
check_sizes(sizes_);
904943
}
905944
// uniform_data_ only valid for low dim tensors
906945
if (sizes.size() <= 4) {

‎backends/vulkan/runtime/api/containers/Tensor.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,11 @@ class vTensorStorage final {
140140

141141
vTensorStorage(Context* const context, const vkapi::VulkanImage& image);
142142

143+
vTensorStorage(
144+
Context* const context,
145+
const vkapi::VulkanBuffer& buffer,
146+
const vkapi::ScalarType dtype);
147+
143148
public:
144149
vTensorStorage(vTensorStorage& other) = delete;
145150
vTensorStorage& operator=(const vTensorStorage& other) = delete;
@@ -211,9 +216,11 @@ class vTensor final {
211216
const utils::GPUMemoryLayout memory_layout = utils::kChannelsPacked,
212217
const bool allocate_memory = true,
213218
const utils::AxisMapLayout axis_map_layout = utils::kDefaultAxisMap,
214-
// When non-null, wrap this image instead of allocating. Stored as a
215-
// VulkanImage copy, which aliases the handle without owning it.
216-
const vkapi::VulkanImage* external_image = nullptr);
219+
// When non-null, wrap this resource instead of allocating. Stored as a
220+
// copy, which aliases the handle without owning it. At most one may be
221+
// set.
222+
const vkapi::VulkanImage* external_image = nullptr,
223+
const vkapi::VulkanBuffer* external_buffer = nullptr);
217224

218225
vTensor(const vTensor& other) = delete;
219226

‎backends/vulkan/runtime/graph/ComputeGraph.cpp‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,26 @@ ValueRef ComputeGraph::add_tensor(
573573
return idx;
574574
}
575575

576+
ValueRef ComputeGraph::add_tensor(
577+
const std::vector<int64_t>& sizes,
578+
const vkapi::ScalarType dtype,
579+
const utils::GPUMemoryLayout memory_layout,
580+
const vkapi::VulkanBuffer& buffer) {
581+
ValueRef idx(static_cast<int>(values_.size()));
582+
check_no_active_value_ptrs();
583+
values_.emplace_back(api::vTensor(
584+
context(),
585+
sizes,
586+
dtype,
587+
utils::kBuffer,
588+
memory_layout,
589+
/*allocate_memory=*/false,
590+
utils::kDefaultAxisMap,
591+
/*external_image=*/nullptr,
592+
&buffer));
593+
return idx;
594+
}
595+
576596
ValueRef ComputeGraph::add_tensor(const vkapi::VulkanImage& image) {
577597
ValueRef idx(static_cast<int>(values_.size()));
578598
check_no_active_value_ptrs();

‎backends/vulkan/runtime/graph/ComputeGraph.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,16 @@ class ComputeGraph final {
790790
const utils::GPUMemoryLayout memory_layout,
791791
const vkapi::VulkanImage& image);
792792

793+
/*
794+
* Wrap an externally owned buffer as a tensor with the given logical sizes.
795+
* The buffer must be large enough for them.
796+
*/
797+
ValueRef add_tensor(
798+
const std::vector<int64_t>& sizes,
799+
const vkapi::ScalarType dtype,
800+
const utils::GPUMemoryLayout memory_layout,
801+
const vkapi::VulkanBuffer& buffer);
802+
793803
/*
794804
* Add a `api::vTensor` value to the graph with the properties of `vref`.
795805
*/

‎backends/vulkan/test/vulkan_compute_api_test.cpp‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,6 +1781,79 @@ TEST_F(VulkanComputeAPITest, test_tensor_over_external_image_does_not_own_it) {
17811781
EXPECT_FALSE(owner.image().is_copy());
17821782
}
17831783

1784+
static vTensor make_buffer_owner() {
1785+
return vTensor(
1786+
context(),
1787+
kExternalSizes,
1788+
vkapi::kFloat,
1789+
utils::kBuffer,
1790+
utils::kWidthPacked);
1791+
}
1792+
1793+
TEST_F(VulkanComputeAPITest, test_tensor_over_external_buffer_keeps_sizes) {
1794+
vTensor owner = make_buffer_owner();
1795+
1796+
vTensor view(
1797+
context(),
1798+
kExternalSizes,
1799+
vkapi::kFloat,
1800+
utils::kBuffer,
1801+
utils::kWidthPacked,
1802+
/*allocate_memory = */ false,
1803+
utils::kDefaultAxisMap,
1804+
/*external_image = */ nullptr,
1805+
&owner.buffer());
1806+
1807+
EXPECT_TRUE(view.sizes() == kExternalSizes);
1808+
EXPECT_TRUE(view.numel() == owner.numel());
1809+
EXPECT_TRUE(view.storage_type() == utils::kBuffer);
1810+
}
1811+
1812+
TEST_F(
1813+
VulkanComputeAPITest,
1814+
test_tensor_over_external_buffer_rejects_overflow) {
1815+
vTensor owner = make_buffer_owner();
1816+
1817+
std::vector<int64_t> too_large = kExternalSizes;
1818+
too_large.back() *= 2;
1819+
1820+
EXPECT_THROW(
1821+
vTensor(
1822+
context(),
1823+
too_large,
1824+
vkapi::kFloat,
1825+
utils::kBuffer,
1826+
utils::kWidthPacked,
1827+
/*allocate_memory = */ false,
1828+
utils::kDefaultAxisMap,
1829+
/*external_image = */ nullptr,
1830+
&owner.buffer()),
1831+
vkapi::Error);
1832+
}
1833+
1834+
TEST_F(VulkanComputeAPITest, test_tensor_over_external_buffer_does_not_own_it) {
1835+
vTensor owner = make_buffer_owner();
1836+
1837+
{
1838+
vTensor view(
1839+
context(),
1840+
kExternalSizes,
1841+
vkapi::kFloat,
1842+
utils::kBuffer,
1843+
utils::kWidthPacked,
1844+
/*allocate_memory = */ false,
1845+
utils::kDefaultAxisMap,
1846+
/*external_image = */ nullptr,
1847+
&owner.buffer());
1848+
1849+
EXPECT_TRUE(view.buffer().is_copy_of(owner.buffer()));
1850+
}
1851+
1852+
context()->flush();
1853+
EXPECT_TRUE(owner.buffer());
1854+
EXPECT_FALSE(owner.buffer().is_copy());
1855+
}
1856+
17841857
TEST(VulkanComputeGraphTest, test_values_scalars) {
17851858
GraphConfig config;
17861859
ComputeGraph graph(config);

0 commit comments

Comments
 (0)