Commit dd0e600a by Nicolas Capens Committed by Nicolas Capens

Associate bug IDs with unimplemented functionality

Also correct uses of UNIMPLEMENTED() for functionality that is not advertised as supported, and add spec quotes to justify and document them. This change also enforces requiring that the UNIMPLEMENTED() macro's format string starts with a bug reference of the form b/###. Note we still have to mention the bug ID in a FIXME() comment as well for 'go/todo' issue tracking (until b/148221114 gets fixed). Bug: b/131243109 Change-Id: Ieb28e31f2c6c80a27b833626538e7e223b5fda08 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/40509 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent 614a4d47
...@@ -1747,11 +1747,11 @@ void Blitter::blit(const vk::Image *src, vk::Image *dst, VkImageBlit region, VkF ...@@ -1747,11 +1747,11 @@ void Blitter::blit(const vk::Image *src, vk::Image *dst, VkImageBlit region, VkF
return; return;
} }
if((region.srcSubresource.layerCount != region.dstSubresource.layerCount) || // Vulkan 1.2 section 18.5. Image Copies with Scaling:
(region.srcSubresource.aspectMask != region.dstSubresource.aspectMask)) // "The layerCount member of srcSubresource and dstSubresource must match"
{ // "The aspectMask member of srcSubresource and dstSubresource must match"
UNIMPLEMENTED("region"); ASSERT(region.srcSubresource.layerCount == region.dstSubresource.layerCount);
} ASSERT(region.srcSubresource.aspectMask == region.dstSubresource.aspectMask);
if(region.dstOffsets[0].x > region.dstOffsets[1].x) if(region.dstOffsets[0].x > region.dstOffsets[1].x)
{ {
...@@ -1890,10 +1890,9 @@ Blitter::CornerUpdateRoutineType Blitter::generateCornerUpdate(const State &stat ...@@ -1890,10 +1890,9 @@ Blitter::CornerUpdateRoutineType Blitter::generateCornerUpdate(const State &stat
ASSERT(state.sourceFormat == state.destFormat); ASSERT(state.sourceFormat == state.destFormat);
ASSERT(state.srcSamples == state.destSamples); ASSERT(state.srcSamples == state.destSamples);
if(state.srcSamples != 1) // Vulkan 1.2: "If samples is not VK_SAMPLE_COUNT_1_BIT, then imageType must be
{ // VK_IMAGE_TYPE_2D, flags must not contain VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT"
UNIMPLEMENTED("state.srcSamples %d", state.srcSamples); ASSERT(state.srcSamples == 1);
}
CornerUpdateFunction function; CornerUpdateFunction function;
{ {
...@@ -1922,11 +1921,7 @@ Blitter::CornerUpdateRoutineType Blitter::generateCornerUpdate(const State &stat ...@@ -1922,11 +1921,7 @@ Blitter::CornerUpdateRoutineType Blitter::generateCornerUpdate(const State &stat
void Blitter::updateBorders(vk::Image *image, const VkImageSubresourceLayers &subresourceLayers) void Blitter::updateBorders(vk::Image *image, const VkImageSubresourceLayers &subresourceLayers)
{ {
if(image->getArrayLayers() < (subresourceLayers.baseArrayLayer + 6)) ASSERT(image->getArrayLayers() >= (subresourceLayers.baseArrayLayer + 6));
{
UNIMPLEMENTED("image->getArrayLayers() %d, baseArrayLayer %d",
image->getArrayLayers(), subresourceLayers.baseArrayLayer);
}
// From Vulkan 1.1 spec, section 11.5. Image Views: // From Vulkan 1.1 spec, section 11.5. Image Views:
// "For cube and cube array image views, the layers of the image view starting // "For cube and cube array image views, the layers of the image view starting
...@@ -1980,10 +1975,9 @@ void Blitter::updateBorders(vk::Image *image, const VkImageSubresourceLayers &su ...@@ -1980,10 +1975,9 @@ void Blitter::updateBorders(vk::Image *image, const VkImageSubresourceLayers &su
VkSampleCountFlagBits samples = image->getSampleCountFlagBits(); VkSampleCountFlagBits samples = image->getSampleCountFlagBits();
State state(format, format, samples, samples, Options{ 0xF }); State state(format, format, samples, samples, Options{ 0xF });
if(samples != VK_SAMPLE_COUNT_1_BIT) // Vulkan 1.2: "If samples is not VK_SAMPLE_COUNT_1_BIT, then imageType must be
{ // VK_IMAGE_TYPE_2D, flags must not contain VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT"
UNIMPLEMENTED("Multi-sampled cube: %d samples", static_cast<int>(samples)); ASSERT(samples == VK_SAMPLE_COUNT_1_BIT);
}
auto cornerUpdateRoutine = getCornerUpdateRoutine(state); auto cornerUpdateRoutine = getCornerUpdateRoutine(state);
if(!cornerUpdateRoutine) if(!cornerUpdateRoutine)
......
...@@ -581,13 +581,13 @@ SpirvShader::EmitResult SpirvShader::EmitFunctionCall(InsnIterator insn, EmitSta ...@@ -581,13 +581,13 @@ SpirvShader::EmitResult SpirvShader::EmitFunctionCall(InsnIterator insn, EmitSta
{ {
if(insnNumber > 1) if(insnNumber > 1)
{ {
UNIMPLEMENTED("Function block number of instructions: %d", insnNumber); UNIMPLEMENTED("b/141246700: Function block number of instructions: %d", insnNumber); // FIXME(b/141246700)
return EmitResult::Continue; return EmitResult::Continue;
} }
if(blockInsn.opcode() != wrapOpKill[insnNumber++]) if(blockInsn.opcode() != wrapOpKill[insnNumber++])
{ {
UNIMPLEMENTED("Function block instruction %d : %s", insnNumber - 1, OpcodeName(blockInsn.opcode()).c_str()); UNIMPLEMENTED("b/141246700: Function block instruction %d : %s", insnNumber - 1, OpcodeName(blockInsn.opcode()).c_str()); // FIXME(b/141246700)
return EmitResult::Continue; return EmitResult::Continue;
} }
......
...@@ -225,7 +225,7 @@ SpirvShader::EmitResult SpirvShader::EmitVariable(InsnIterator insn, EmitState * ...@@ -225,7 +225,7 @@ SpirvShader::EmitResult SpirvShader::EmitVariable(InsnIterator insn, EmitState *
Object::ID initializerId = insn.word(4); Object::ID initializerId = insn.word(4);
if(getObject(initializerId).kind != Object::Kind::Constant) if(getObject(initializerId).kind != Object::Kind::Constant)
{ {
UNIMPLEMENTED("Non-constant initializers not yet implemented"); UNIMPLEMENTED("b/148241854: Non-constant initializers not yet implemented"); // FIXME(b/148241854)
} }
switch(objectTy.storageClass) switch(objectTy.storageClass)
......
...@@ -20,6 +20,8 @@ ...@@ -20,6 +20,8 @@
#include <assert.h> #include <assert.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <cctype>
#include <string> #include <string>
#if !defined(TRACE_OUTPUT_FILE) #if !defined(TRACE_OUTPUT_FILE)
...@@ -110,11 +112,13 @@ void trace_assert(const char *format, ...) CHECK_PRINTF_ARGS; ...@@ -110,11 +112,13 @@ void trace_assert(const char *format, ...) CHECK_PRINTF_ARGS;
} \ } \
} while(0) } while(0)
// A macro to indicate functionality currently unimplemented for a feature // A macro to indicate functionality currently unimplemented, for a feature advertised
// advertised as supported. For unsupported features not advertised as supported // as supported. Since this is a bug, a bug ID must be provided, in b/### format.
// use UNSUPPORTED(). // For unimplemented functionality not advertised as supported, use UNSUPPORTED() instead.
#undef UNIMPLEMENTED #undef UNIMPLEMENTED
#define UNIMPLEMENTED(format, ...) DABORT("UNIMPLEMENTED: " format, ##__VA_ARGS__) #define UNIMPLEMENTED(format, ...) \
DABORT("UNIMPLEMENTED: " format, ##__VA_ARGS__); \
static_assert(format[0] == 'b' && format[1] == '/' && format[2] >= '0' && format[2] <= '9', "explanation must start with bug reference in b/### format")
// A macro to indicate unsupported functionality. // A macro to indicate unsupported functionality.
// This should be called when a Vulkan / SPIR-V feature is attempted to be used, // This should be called when a Vulkan / SPIR-V feature is attempted to be used,
......
...@@ -13,6 +13,8 @@ ...@@ -13,6 +13,8 @@
// limitations under the License. // limitations under the License.
#include "VkDebug.hpp" #include "VkDebug.hpp"
#include "VkStringify.hpp"
#include <android/hardware_buffer.h> #include <android/hardware_buffer.h>
#include <errno.h> #include <errno.h>
...@@ -55,7 +57,7 @@ public: ...@@ -55,7 +57,7 @@ public:
if(exportInfo->handleTypes != VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID) if(exportInfo->handleTypes != VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID)
{ {
UNIMPLEMENTED("exportInfo->handleTypes"); UNSUPPORTED("VkExportMemoryAllocateInfo::handleTypes %d", int(exportInfo->handleTypes));
} }
exportAhb = true; exportAhb = true;
} }
...@@ -70,7 +72,8 @@ public: ...@@ -70,7 +72,8 @@ public:
} }
break; break;
default:; default:
WARN("VkMemoryAllocateInfo->pNext sType = %s", vk::Stringify(createInfo->sType).c_str());
} }
createInfo = createInfo->pNext; createInfo = createInfo->pNext;
} }
...@@ -146,7 +149,7 @@ public: ...@@ -146,7 +149,7 @@ public:
static VkResult getAhbProperties(const struct AHardwareBuffer *buffer, VkAndroidHardwareBufferPropertiesANDROID *pProperties) static VkResult getAhbProperties(const struct AHardwareBuffer *buffer, VkAndroidHardwareBufferPropertiesANDROID *pProperties)
{ {
UNIMPLEMENTED("getAhbProperties"); UNIMPLEMENTED("b/141698760: getAhbProperties"); // FIXME(b/141698760)
return VK_SUCCESS; return VK_SUCCESS;
} }
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
// limitations under the License. // limitations under the License.
#include "VkDebug.hpp" #include "VkDebug.hpp"
#include "VkStringify.hpp"
#include "System/Linux/MemFd.hpp" #include "System/Linux/MemFd.hpp"
#include <errno.h> #include <errno.h>
...@@ -47,7 +48,7 @@ public: ...@@ -47,7 +48,7 @@ public:
if(importInfo->handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT) if(importInfo->handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT)
{ {
UNIMPLEMENTED("importInfo->handleType"); UNSUPPORTED("VkImportMemoryFdInfoKHR::handleType %d", int(importInfo->handleType));
} }
importFd = true; importFd = true;
fd = importInfo->fd; fd = importInfo->fd;
...@@ -59,13 +60,14 @@ public: ...@@ -59,13 +60,14 @@ public:
if(exportInfo->handleTypes != VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT) if(exportInfo->handleTypes != VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT)
{ {
UNIMPLEMENTED("exportInfo->handleTypes"); UNSUPPORTED("VkExportMemoryAllocateInfo::handleTypes %d", int(exportInfo->handleTypes));
} }
exportFd = true; exportFd = true;
} }
break; break;
default:; default:
WARN("VkMemoryAllocateInfo->pNext sType = %s", vk::Stringify(createInfo->sType).c_str());
} }
createInfo = createInfo->pNext; createInfo = createInfo->pNext;
} }
......
...@@ -892,21 +892,15 @@ void Image::clear(void *pixelData, VkFormat pixelFormat, const vk::Format &viewF ...@@ -892,21 +892,15 @@ void Image::clear(void *pixelData, VkFormat pixelFormat, const vk::Format &viewF
void Image::clear(const VkClearColorValue &color, const VkImageSubresourceRange &subresourceRange) void Image::clear(const VkClearColorValue &color, const VkImageSubresourceRange &subresourceRange)
{ {
if(!(subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT)) ASSERT(subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT);
{
UNIMPLEMENTED("aspectMask");
}
device->getBlitter()->clear((void *)color.float32, getClearFormat(), this, format, subresourceRange); device->getBlitter()->clear((void *)color.float32, getClearFormat(), this, format, subresourceRange);
} }
void Image::clear(const VkClearDepthStencilValue &color, const VkImageSubresourceRange &subresourceRange) void Image::clear(const VkClearDepthStencilValue &color, const VkImageSubresourceRange &subresourceRange)
{ {
if((subresourceRange.aspectMask & ~(VK_IMAGE_ASPECT_DEPTH_BIT | ASSERT((subresourceRange.aspectMask & ~(VK_IMAGE_ASPECT_DEPTH_BIT |
VK_IMAGE_ASPECT_STENCIL_BIT)) != 0) VK_IMAGE_ASPECT_STENCIL_BIT)) == 0);
{
UNIMPLEMENTED("aspectMask");
}
if(subresourceRange.aspectMask & VK_IMAGE_ASPECT_DEPTH_BIT) if(subresourceRange.aspectMask & VK_IMAGE_ASPECT_DEPTH_BIT)
{ {
...@@ -925,12 +919,9 @@ void Image::clear(const VkClearDepthStencilValue &color, const VkImageSubresourc ...@@ -925,12 +919,9 @@ void Image::clear(const VkClearDepthStencilValue &color, const VkImageSubresourc
void Image::clear(const VkClearValue &clearValue, const vk::Format &viewFormat, const VkRect2D &renderArea, const VkImageSubresourceRange &subresourceRange) void Image::clear(const VkClearValue &clearValue, const vk::Format &viewFormat, const VkRect2D &renderArea, const VkImageSubresourceRange &subresourceRange)
{ {
if(!((subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT) || ASSERT((subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT) ||
(subresourceRange.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | (subresourceRange.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT |
VK_IMAGE_ASPECT_STENCIL_BIT)))) VK_IMAGE_ASPECT_STENCIL_BIT)));
{
UNIMPLEMENTED("subresourceRange");
}
if(subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT) if(subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT)
{ {
......
...@@ -75,6 +75,7 @@ void ImageView::destroy(const VkAllocationCallbacks *pAllocator) ...@@ -75,6 +75,7 @@ void ImageView::destroy(const VkAllocationCallbacks *pAllocator)
{ {
} }
// Vulkan 1.2 Table 8. Image and image view parameter compatibility requirements
bool ImageView::imageTypesMatch(VkImageType imageType) const bool ImageView::imageTypesMatch(VkImageType imageType) const
{ {
uint32_t imageArrayLayers = image->getArrayLayers(); uint32_t imageArrayLayers = image->getArrayLayers();
...@@ -118,15 +119,8 @@ void ImageView::clear(const VkClearValue &clearValue, const VkImageAspectFlags a ...@@ -118,15 +119,8 @@ void ImageView::clear(const VkClearValue &clearValue, const VkImageAspectFlags a
{ {
// Note: clearing ignores swizzling, so components is ignored. // Note: clearing ignores swizzling, so components is ignored.
if(!imageTypesMatch(image->getImageType())) ASSERT(imageTypesMatch(image->getImageType()));
{ ASSERT(format.isCompatible(image->getFormat()));
UNIMPLEMENTED("imageTypesMatch");
}
if(!format.isCompatible(image->getFormat()))
{
UNIMPLEMENTED("incompatible formats");
}
VkImageSubresourceRange sr = subresourceRange; VkImageSubresourceRange sr = subresourceRange;
sr.aspectMask = aspectMask; sr.aspectMask = aspectMask;
...@@ -137,15 +131,8 @@ void ImageView::clear(const VkClearValue &clearValue, const VkImageAspectFlags a ...@@ -137,15 +131,8 @@ void ImageView::clear(const VkClearValue &clearValue, const VkImageAspectFlags a
{ {
// Note: clearing ignores swizzling, so components is ignored. // Note: clearing ignores swizzling, so components is ignored.
if(!imageTypesMatch(image->getImageType())) ASSERT(imageTypesMatch(image->getImageType()));
{ ASSERT(format.isCompatible(image->getFormat()));
UNIMPLEMENTED("imageTypesMatch");
}
if(!format.isCompatible(image->getFormat()))
{
UNIMPLEMENTED("incompatible formats");
}
VkImageSubresourceRange sr; VkImageSubresourceRange sr;
sr.aspectMask = aspectMask; sr.aspectMask = aspectMask;
...@@ -173,7 +160,7 @@ void ImageView::resolve(ImageView *resolveAttachment, int layer) ...@@ -173,7 +160,7 @@ void ImageView::resolve(ImageView *resolveAttachment, int layer)
{ {
if((subresourceRange.levelCount != 1) || (resolveAttachment->subresourceRange.levelCount != 1)) if((subresourceRange.levelCount != 1) || (resolveAttachment->subresourceRange.levelCount != 1))
{ {
UNIMPLEMENTED("levelCount"); UNIMPLEMENTED("b/148242443: levelCount != 1"); // FIXME(b/148242443)
} }
VkImageCopy region; VkImageCopy region;
...@@ -201,7 +188,7 @@ void ImageView::resolve(ImageView *resolveAttachment) ...@@ -201,7 +188,7 @@ void ImageView::resolve(ImageView *resolveAttachment)
{ {
if((subresourceRange.levelCount != 1) || (resolveAttachment->subresourceRange.levelCount != 1)) if((subresourceRange.levelCount != 1) || (resolveAttachment->subresourceRange.levelCount != 1))
{ {
UNIMPLEMENTED("levelCount"); UNIMPLEMENTED("b/148242443: levelCount != 1"); // FIXME(b/148242443)
} }
VkImageCopy region; VkImageCopy region;
......
...@@ -189,7 +189,7 @@ void QueryPool::begin(uint32_t query, VkQueryControlFlags flags) ...@@ -189,7 +189,7 @@ void QueryPool::begin(uint32_t query, VkQueryControlFlags flags)
if(flags != 0) if(flags != 0)
{ {
UNIMPLEMENTED("vkCmdBeginQuery::flags %d", int(flags)); UNSUPPORTED("vkCmdBeginQuery::flags %d", int(flags));
} }
pool[query].prepare(type); pool[query].prepare(type);
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "VkSemaphore.hpp" #include "VkSemaphore.hpp"
#include "VkConfig.h" #include "VkConfig.h"
#include "VkStringify.hpp"
#if SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD #if SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD
# if defined(__linux__) || defined(__ANDROID__) # if defined(__linux__) || defined(__ANDROID__)
...@@ -50,15 +51,20 @@ struct SemaphoreCreateInfo ...@@ -50,15 +51,20 @@ struct SemaphoreCreateInfo
for(const auto *nextInfo = reinterpret_cast<const VkBaseInStructure *>(pCreateInfo->pNext); for(const auto *nextInfo = reinterpret_cast<const VkBaseInStructure *>(pCreateInfo->pNext);
nextInfo != nullptr; nextInfo = nextInfo->pNext) nextInfo != nullptr; nextInfo = nextInfo->pNext)
{ {
if(nextInfo->sType == VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO) switch(nextInfo->sType)
{ {
const auto *exportInfo = reinterpret_cast<const VkExportSemaphoreCreateInfo *>(nextInfo); case VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO:
exportSemaphore = true;
if(exportInfo->handleTypes != Semaphore::External::kExternalSemaphoreHandleType)
{ {
UNIMPLEMENTED("exportInfo->handleTypes"); const auto *exportInfo = reinterpret_cast<const VkExportSemaphoreCreateInfo *>(nextInfo);
exportSemaphore = true;
if(exportInfo->handleTypes != Semaphore::External::kExternalSemaphoreHandleType)
{
UNSUPPORTED("exportInfo->handleTypes %d", int(exportInfo->handleTypes));
}
} }
break; break;
default:
WARN("nextInfo->sType = %s", vk::Stringify(nextInfo->sType).c_str());
} }
} }
} }
......
...@@ -273,7 +273,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateInstance(const VkInstanceCreateInfo *pCre ...@@ -273,7 +273,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateInstance(const VkInstanceCreateInfo *pCre
if(pCreateInfo->enabledLayerCount != 0) if(pCreateInfo->enabledLayerCount != 0)
{ {
UNIMPLEMENTED("pCreateInfo->enabledLayerCount != 0"); UNIMPLEMENTED("b/148240133: pCreateInfo->enabledLayerCount != 0"); // FIXME(b/148240133)
} }
uint32_t extensionPropertiesCount = sizeof(instanceExtensionProperties) / sizeof(instanceExtensionProperties[0]); uint32_t extensionPropertiesCount = sizeof(instanceExtensionProperties) / sizeof(instanceExtensionProperties[0]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment