Skip to content
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.

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

[vulkan] Change command list submit error message & misc device API cleanups #7395

Merged
merged 2 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
82 changes: 35 additions & 47 deletions taichi/rhi/vulkan/vulkan_device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,10 @@ void VulkanPipeline::create_pipeline_layout() {
}

void VulkanPipeline::create_compute_pipeline(const Params &params) {
char msg_buf[512];
RHI_DEBUG_SNPRINTF(msg_buf, sizeof(msg_buf), "Compiling Vulkan pipeline %s",
params.name.data());
RHI_LOG_DEBUG(msg_buf);
std::array<char, 512> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Compiling Vulkan pipeline %s", params.name.data());
RHI_LOG_DEBUG(msg_buf.data());
pipeline_ = vkapi::create_compute_pipeline(device_, 0, shader_stages_[0],
pipeline_layout_, params.cache);
}
Expand Down Expand Up @@ -554,11 +554,12 @@ void VulkanPipeline::create_graphics_pipeline(
if (raster_params.blending.size()) {
if (raster_params.blending.size() != color_blending.attachmentCount) {
std::array<char, 256> buf;
snprintf(buf.data(), buf.size(),
"RasterParams::blending (size=%u) must either be zero sized "
"or match the number of fragment shader outputs (size=%u).",
uint32_t(raster_params.blending.size()),
uint32_t(color_blending.attachmentCount));
RHI_DEBUG_SNPRINTF(
buf.data(), buf.size(),
"RasterParams::blending (size=%u) must either be zero sized "
"or match the number of fragment shader outputs (size=%u).",
uint32_t(raster_params.blending.size()),
uint32_t(color_blending.attachmentCount));
RHI_LOG_ERROR(buf.data());
RHI_ASSERT(false);
}
Expand Down Expand Up @@ -931,17 +932,19 @@ RhiResult VulkanCommandList::bind_shader_resources(ShaderResourceSet *res,
VulkanResourceSet &set_template = templates.at(set_index);

for (const auto &template_binding : set_template.get_bindings()) {
char msg[512];
snprintf(msg, 512, "Template binding %d: (VkDescriptorType) %d",
template_binding.first, template_binding.second.type);
RHI_LOG_ERROR(msg);
std::array<char, 256> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Template binding %d: (VkDescriptorType) %d",
template_binding.first, template_binding.second.type);
RHI_LOG_ERROR(msg_buf.data());
}

for (const auto &binding : set->get_bindings()) {
char msg[512];
snprintf(msg, 512, "Binding %d: (VkDescriptorType) %d", binding.first,
binding.second.type);
RHI_LOG_ERROR(msg);
std::array<char, 256> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Binding %d: (VkDescriptorType) %d", binding.first,
binding.second.type);
RHI_LOG_ERROR(msg_buf.data());
}

return RhiResult::invalid_usage;
Expand Down Expand Up @@ -1612,7 +1615,6 @@ VulkanDevice::~VulkanDevice() {
compute_streams_.reset();
graphics_streams_.reset();

framebuffer_pools_.clear();
renderpass_pools_.clear();
desc_set_layouts_.clear();
desc_pool_ = nullptr;
Expand Down Expand Up @@ -1826,10 +1828,11 @@ RhiResult VulkanDevice::map_internal(AllocationInternal &alloc_int,
"wrapper)");
return RhiResult::invalid_usage;
} else if (res != VK_SUCCESS) {
char msg_buf[256];
snprintf(msg_buf, sizeof(msg_buf),
"failed to map memory for unknown reasons. VkResult = %d", res);
RHI_LOG_ERROR(msg_buf);
std::array<char, 256> msg_buf;
RHI_DEBUG_SNPRINTF(
msg_buf.data(), msg_buf.size(),
"failed to map memory for unknown reasons. VkResult = %d", res);
RHI_LOG_ERROR(msg_buf.data());
return RhiResult::error;
}

Expand Down Expand Up @@ -2021,23 +2024,12 @@ StreamSemaphore VulkanStream::submit(
auto fence = vkapi::create_fence(buffer->device, 0);

// Resource tracking, check previously submitted commands
// FIXME: Figure out why it doesn't work
/*
std::remove_if(submitted_cmdbuffers_.begin(), submitted_cmdbuffers_.end(),
[&](const TrackedCmdbuf &tracked) {
// If fence is signaled, cmdbuf has completed
VkResult res =
vkGetFenceStatus(buffer->device, tracked.fence->fence);
return res == VK_SUCCESS;
});
*/

submitted_cmdbuffers_.push_back(TrackedCmdbuf{fence, buffer});

BAIL_ON_VK_BAD_RESULT_NO_RETURN(
vkQueueSubmit(queue_, /*submitCount=*/1, &submit_info,
/*fence=*/fence->fence),
"failed to submit command buffer");
"Vulkan device might be lost (vkQueueSubmit failed)");

return std::make_shared<VulkanStreamSemaphoreObject>(semaphore);
}
Expand All @@ -2053,9 +2045,6 @@ StreamSemaphore VulkanStream::submit_synced(
void VulkanStream::command_sync() {
vkQueueWaitIdle(queue_);

VkPhysicalDeviceProperties props{};
vkGetPhysicalDeviceProperties(device_.vk_physical_device(), &props);

device_.profiler_sync();

submitted_cmdbuffers_.clear();
Expand Down Expand Up @@ -2130,15 +2119,13 @@ VulkanDevice::get_vk_image(const DeviceAllocation &alloc) const {

vkapi::IVkFramebuffer VulkanDevice::get_framebuffer(
const VulkanFramebufferDesc &desc) {
if (framebuffer_pools_.find(desc) != framebuffer_pools_.end()) {
return framebuffer_pools_.at(desc);
}

// We won't pool framebuffer and resuse it, as doing so requires hashing the
// referenced IVkImageView objects, which might destruct unless we hold strong
// references. Thus doing so is way too ugly, and Vulkan is moving towards
// dynamic rendering anyways.
vkapi::IVkFramebuffer framebuffer = vkapi::create_framebuffer(
0, desc.renderpass, desc.attachments, desc.width, desc.height, 1);

framebuffer_pools_.insert({desc, framebuffer});

return framebuffer;
}

Expand Down Expand Up @@ -2688,10 +2675,11 @@ void VulkanSurface::create_swap_chain() {
std::max(capabilities.minImageExtent.height,
std::min(capabilities.maxImageExtent.height, extent.height));
{
char msg_buf[512];
RHI_DEBUG_SNPRINTF(msg_buf, sizeof(msg_buf), "Creating suface of %u x %u",
extent.width, extent.height);
RHI_LOG_DEBUG(msg_buf);
std::array<char, 512> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Creating suface of %u x %u", extent.width,
extent.height);
RHI_LOG_DEBUG(msg_buf.data());
}
VkImageUsageFlags usage =
VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
Expand Down
29 changes: 5 additions & 24 deletions taichi/rhi/vulkan/vulkan_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,13 @@ struct VulkanRenderPassDesc {

struct RenderPassDescHasher {
std::size_t operator()(const VulkanRenderPassDesc &desc) const {
// TODO: Come up with a better hash
size_t hash = 0;
size_t hash = std::hash<uint64_t>()((uint64_t(desc.depth_attachment) << 1) |
uint64_t(desc.clear_depth));
for (auto &pair : desc.color_attachments) {
hash ^= (size_t(pair.first) + pair.second);
hash = (hash << 3) || (hash >> 61);
size_t hash_pair = std::hash<uint64_t>()((uint64_t(pair.first) << 1) |
uint64_t(pair.second));
rhi_impl::hash_combine(hash, hash_pair);
}
hash ^= (size_t(desc.depth_attachment) + desc.clear_depth);
hash = (hash << 3) || (hash >> 61);
return hash;
}
};
Expand All @@ -75,20 +74,6 @@ struct VulkanFramebufferDesc {
}
};

struct FramebufferDescHasher {
std::size_t operator()(const VulkanFramebufferDesc &desc) const {
size_t hash = 0;
for (auto &view : desc.attachments) {
hash ^= size_t(view->view);
hash = (hash << 3) || (hash >> 61);
}
hash ^= desc.width;
hash ^= desc.height;
hash ^= size_t(desc.renderpass->renderpass);
return hash;
}
};

class VulkanResourceSet : public ShaderResourceSet {
public:
struct Buffer {
Expand Down Expand Up @@ -808,10 +793,6 @@ class TI_DLL_EXPORT VulkanDevice : public GraphicsDevice {
vkapi::IVkRenderPass,
RenderPassDescHasher>
renderpass_pools_;
unordered_map<VulkanFramebufferDesc,
vkapi::IVkFramebuffer,
FramebufferDescHasher>
framebuffer_pools_;

// Descriptors / Layouts / Pools
unordered_map<VulkanResourceSet,
Expand Down