Don't check kernel entry-point signatures.
authorFlorian Ziesche <florian.ziesche@gmail.com>
Tue, 1 Mar 2016 18:56:14 +0000 (19:56 +0100)
committerDejan Mircevski <deki@google.com>
Wed, 2 Mar 2016 20:27:26 +0000 (15:27 -0500)
Recognize SpvOpInBoundsPtrAccessChain and SpvOpPtrAccessChain as opcodes
returning a pointer.

 * spvOpcodeIsPointer: recognize SpvOpInBoundsPtrAccessChain and SpvOpPtrAccessChain as opcodes returning a pointer

 * isValid<SpvOpEntryPoint>: don't check kernel function signatures (these don't have to be 'void main(void)')

 * added tests for kernel OpEntryPoint, OpInBoundsPtrAccessChain and OpPtrAccessChain, as well as facilities to actually test kernel/OpenCL SPIR-V

 * fixed pow and pown specification (both should take 2 parameters), spec bug reported at https://www.khronos.org/bugzilla/show_bug.cgi?id=1469

 * use ASSERT_TRUE instead of ASSERT_EQ

 * added pow and pown test (pow(val, 2.0f) and pown(val, 3))

Revert " * fixed pow and pown specification (both should take 2 parameters), spec bug reported at https://www.khronos.org/bugzilla/show_bug.cgi?id=1469"

This reverts commit c3d5a87e73334b25aa4278964eea6d1c7625cd81.

Revert " * added pow and pown test (pow(val, 2.0f) and pown(val, 3))"

This reverts commit 7624aec720a51dc212704dc92eff6ae3634d2d6c.

source/opcode.cpp
source/validate_id.cpp
test/ValidateID.cpp

index 6e9d240..1a37a6a 100644 (file)
@@ -470,7 +470,9 @@ int32_t spvOpcodeIsPointer(const SpvOp opcode) {
   switch (opcode) {
     case SpvOpVariable:
     case SpvOpAccessChain:
+    case SpvOpPtrAccessChain:
     case SpvOpInBoundsAccessChain:
+    case SpvOpInBoundsPtrAccessChain:
     case SpvOpFunctionParameter:
       return true;
     default:
index 4668504..513f257 100644 (file)
@@ -188,14 +188,18 @@ bool idUsage::isValid<SpvOpEntryPoint>(const spv_instruction_t* inst,
                           << "' is not a function.";
     return false;
   }
-  // TODO: Check the entry point signature is void main(void), may be subject
-  // to change
-  auto entryPointType = usedefs_.FindDef(entryPoint.second.words[4]);
-  if (!entryPointType.first || 3 != entryPointType.second.words.size()) {
-    DIAG(entryPointIndex) << "OpEntryPoint Entry Point <id> '"
-                          << inst->words[entryPointIndex]
-                          << "'s function parameter count is not zero.";
-    return false;
+  // don't check kernel function signatures
+  auto executionModel = inst->words[1];
+  if (executionModel != SpvExecutionModelKernel) {
+    // TODO: Check the entry point signature is void main(void), may be subject
+    // to change
+    auto entryPointType = usedefs_.FindDef(entryPoint.second.words[4]);
+    if (!entryPointType.first || 3 != entryPointType.second.words.size()) {
+      DIAG(entryPointIndex) << "OpEntryPoint Entry Point <id> '"
+                            << inst->words[entryPointIndex]
+                            << "'s function parameter count is not zero.";
+      return false;
+    }
   }
   auto returnType = usedefs_.FindDef(entryPoint.second.type_id);
   if (!returnType.first || SpvOpTypeVoid != returnType.second.opcode) {
index 446dbd0..171ec97 100644 (file)
@@ -52,6 +52,23 @@ const char kGLSL450MemoryModel[] = R"(
      OpMemoryModel Logical GLSL450
 )";
 
+const char kOpenCLMemoryModel32[] = R"(
+     OpCapability Addresses
+     OpCapability Linkage
+     OpCapability Kernel
+%1 = OpExtInstImport "OpenCL.std"
+     OpMemoryModel Physical32 OpenCL
+)";
+
+const char kOpenCLMemoryModel64[] = R"(
+     OpCapability Addresses
+     OpCapability Linkage
+     OpCapability Kernel
+     OpCapability Int64
+%1 = OpExtInstImport "OpenCL.std"
+     OpMemoryModel Physical64 OpenCL
+)";
+
 #define CHECK(str, expected)                                                   \
   spv_diagnostic diagnostic;                                                   \
   spv_context context = spvContextCreate();                                    \
@@ -71,6 +88,28 @@ const char kGLSL450MemoryModel[] = R"(
   ASSERT_EQ(expected, result);                                                 \
   spvContextDestroy(context);
 
+#define CHECK_KERNEL(str, expected, bitness)                                   \
+  ASSERT_TRUE(bitness == 32 || bitness == 64);                                 \
+  spv_diagnostic diagnostic;                                                   \
+  spv_context context = spvContextCreate();                                    \
+  std::string kernel = std::string(bitness == 32 ?                             \
+                                   kOpenCLMemoryModel32 :                      \
+                                   kOpenCLMemoryModel64) + str;                \
+  spv_result_t error = spvTextToBinary(context, kernel.c_str(), kernel.size(), \
+                                       &binary, &diagnostic);                  \
+  if (error) {                                                                 \
+    spvDiagnosticPrint(diagnostic);                                            \
+    spvDiagnosticDestroy(diagnostic);                                          \
+    ASSERT_EQ(SPV_SUCCESS, error);                                             \
+  }                                                                            \
+  spv_result_t result = spvValidate(context, get_const_binary(), &diagnostic); \
+  if (SPV_SUCCESS != result) {                                                 \
+    spvDiagnosticPrint(diagnostic);                                            \
+    spvDiagnosticDestroy(diagnostic);                                          \
+  }                                                                            \
+  ASSERT_EQ(expected, result);                                                 \
+  spvContextDestroy(context);
+
 // TODO: OpUndef
 
 TEST_F(ValidateID, OpName) {
@@ -1476,6 +1515,71 @@ TEST_F(ValidateID, UndefinedIdMemSem) {
   CHECK(spirv, SPV_ERROR_INVALID_ID);
 }
 
+TEST_F(ValidateID, KernelOpEntryPointAndOpInBoundsPtrAccessChainGood) {
+  const char* spirv = R"(
+      OpEntryPoint Kernel %2 "simple_kernel"
+      OpSource OpenCL_C 200000
+      OpDecorate %3 BuiltIn GlobalInvocationId
+      OpDecorate %3 Constant
+      OpDecorate %4 FuncParamAttr NoCapture
+      OpDecorate %3 LinkageAttributes "__spirv_GlobalInvocationId" Import
+ %5 = OpTypeInt 32 0
+ %6 = OpTypeVector %5 3
+ %7 = OpTypePointer UniformConstant %6
+ %3 = OpVariable %7 UniformConstant
+ %8 = OpTypeVoid
+ %9 = OpTypeStruct %5
+%10 = OpTypePointer CrossWorkgroup %9
+%11 = OpTypeFunction %8 %10
+%12 = OpConstant %5 0
+%13 = OpTypePointer CrossWorkgroup %5
+%14 = OpConstant %5 42
+ %2 = OpFunction %8 None %11
+ %4 = OpFunctionParameter %10
+%15 = OpLabel
+%16 = OpLoad %6 %3 Aligned 0
+%17 = OpCompositeExtract %5 %16 0
+%18 = OpInBoundsPtrAccessChain %13 %4 %17 %12
+      OpStore %18 %14 Aligned 4
+      OpReturn
+      OpFunctionEnd)";
+  CHECK_KERNEL(spirv, SPV_SUCCESS, 32);
+}
+
+TEST_F(ValidateID, OpPtrAccessChainGood) {
+  const char* spirv = R"(
+      OpEntryPoint Kernel %2 "another_kernel"
+      OpSource OpenCL_C 200000
+      OpDecorate %3 BuiltIn GlobalInvocationId
+      OpDecorate %3 Constant
+      OpDecorate %4 FuncParamAttr NoCapture
+      OpDecorate %3 LinkageAttributes "__spirv_GlobalInvocationId" Import
+ %5 = OpTypeInt 64 0
+ %6 = OpTypeVector %5 3
+ %7 = OpTypePointer UniformConstant %6
+ %3 = OpVariable %7 UniformConstant
+ %8 = OpTypeVoid
+ %9 = OpTypeInt 32 0
+%10 = OpTypeStruct %9
+%11 = OpTypePointer CrossWorkgroup %10
+%12 = OpTypeFunction %8 %11
+%13 = OpConstant %5 4294967295
+%14 = OpConstant %9 0
+%15 = OpTypePointer CrossWorkgroup %9
+%16 = OpConstant %9 42
+ %2 = OpFunction %8 None %12
+ %4 = OpFunctionParameter %11
+%17 = OpLabel
+%18 = OpLoad %6 %3 Aligned 0
+%19 = OpCompositeExtract %5 %18 0
+%20 = OpBitwiseAnd %5 %19 %13
+%21 = OpPtrAccessChain %15 %4 %20 %14
+      OpStore %21 %16 Aligned 4
+      OpReturn
+      OpFunctionEnd)";
+  CHECK_KERNEL(spirv, SPV_SUCCESS, 64);
+}
+
 // TODO: OpLifetimeStart
 // TODO: OpLifetimeStop
 // TODO: OpAtomicInit