From 8d18a0cd12325d289263896f965445115d3750a0 Mon Sep 17 00:00:00 2001 From: Compute-Runtime-Validation Date: Thu, 29 Dec 2022 06:18:48 +0100 Subject: [PATCH] Revert "feat(ocl): Generate minimal set of args info" This reverts commit ea6c91ecccae57755adcc713a9bf410bf6499613. Signed-off-by: Compute-Runtime-Validation --- opencl/source/kernel/kernel.cpp | 4 +- .../source/program/process_device_binary.cpp | 131 ------------------ opencl/source/program/program.h | 2 - .../kernel/kernel_arg_info_tests.cpp | 6 + opencl/test/unit_test/mocks/mock_program.h | 11 -- .../test/unit_test/program/program_tests.cpp | 78 ----------- .../device_binary_format/zebin_decoder.cpp | 6 +- shared/source/kernel/kernel_arg_descriptor.h | 23 +-- 8 files changed, 11 insertions(+), 250 deletions(-) diff --git a/opencl/source/kernel/kernel.cpp b/opencl/source/kernel/kernel.cpp index 85b8104d1a..b88de768c2 100644 --- a/opencl/source/kernel/kernel.cpp +++ b/opencl/source/kernel/kernel.cpp @@ -489,7 +489,9 @@ cl_int Kernel::getArgInfo(cl_uint argIndex, cl_kernel_arg_info paramName, size_t } program->callPopulateZebinExtendedArgsMetadataOnce(clDevice.getRootDeviceIndex()); - program->callGenerateDefaultExtendedArgsMetadataOnce(clDevice.getRootDeviceIndex()); + if (kernelInfo.kernelDescriptor.explicitArgsExtendedMetadata.empty()) { + return CL_KERNEL_ARG_INFO_NOT_AVAILABLE; + } const auto &argTraits = args[argIndex].getTraits(); const auto &argMetadata = kernelInfo.kernelDescriptor.explicitArgsExtendedMetadata[argIndex]; diff --git a/opencl/source/program/process_device_binary.cpp b/opencl/source/program/process_device_binary.cpp index dfb3c5ffb6..c46582a6fa 100644 --- a/opencl/source/program/process_device_binary.cpp +++ b/opencl/source/program/process_device_binary.cpp @@ -381,135 +381,4 @@ void Program::callPopulateZebinExtendedArgsMetadataOnce(uint32_t rootDeviceIndex }; std::call_once(extractAndDecodeMetadataOnce, extractAndDecodeMetadata); } - -void Program::callGenerateDefaultExtendedArgsMetadataOnce(uint32_t rootDeviceIndex) { - auto ensureTypeNone = [](ArgTypeTraits &typeTraits) -> void { - typeTraits.typeQualifiers.constQual = false; - typeTraits.typeQualifiers.pipeQual = false; - typeTraits.typeQualifiers.restrictQual = false; - typeTraits.typeQualifiers.unknownQual = false; - typeTraits.typeQualifiers.volatileQual = false; - }; - - auto getTypeName = [](std::string &type, size_t argSize) -> void { - switch (argSize) { - case 1: - type = std::string("char"); - break; - case 2: - type = std::string("short"); - break; - case 4: - type = std::string("int"); - break; - case 8: - type = std::string("long"); - break; - case 12: - type = std::string("int3"); - break; - case 16: - type = std::string("long2"); - break; - case 24: - type = std::string("long3"); - break; - case 32: - type = std::string("long4"); - break; - case 64: - type = std::string("long8"); - break; - case 128: - type = std::string("long16"); - break; - default: - break; - } - }; - - auto &buildInfo = this->buildInfos[rootDeviceIndex]; - auto generateDefaultMetadata = [&]() { - for (const auto &kernelInfo : buildInfo.kernelInfoArray) { - if (false == kernelInfo->kernelDescriptor.explicitArgsExtendedMetadata.empty()) { - continue; - } - size_t argIndex = 0u; - kernelInfo->kernelDescriptor.explicitArgsExtendedMetadata.resize(kernelInfo->kernelDescriptor.payloadMappings.explicitArgs.size()); - for (auto &kernelArg : kernelInfo->kernelDescriptor.payloadMappings.explicitArgs) { - ArgTypeMetadataExtended argMetadataExtended; - auto &argTypeTraits = kernelArg.getTraits(); - argMetadataExtended.argName = std::string("arg" + std::to_string(argIndex)); - - if (kernelArg.is()) { - const auto &argAsValue = kernelArg.as(false); - - uint16_t maxSourceOffset = 0u, elemSize = 0u; - for (const auto &elem : argAsValue.elements) { - if (maxSourceOffset <= elem.sourceOffset) { - maxSourceOffset = elem.sourceOffset; - elemSize = elem.size; - } - } - if (maxSourceOffset != 0u) { - argMetadataExtended.type = std::string("char[" + std::to_string(maxSourceOffset + elemSize) + "]"); - } else { - getTypeName(argMetadataExtended.type, elemSize); - } - ensureTypeNone(argTypeTraits); - argTypeTraits.addressQualifier = KernelArgMetadata::AddrPrivate; - argTypeTraits.accessQualifier = KernelArgMetadata::AccessNone; - } else if (kernelArg.is()) { - argMetadataExtended.type = std::string("void*"); - } else if (kernelArg.is()) { - const auto &argAsImage = kernelArg.as(false); - switch (argAsImage.imageType) { - case NEOImageType::ImageTypeBuffer: - argMetadataExtended.type = std::string("image1d_buffer_t"); - break; - case NEOImageType::ImageType1D: - argMetadataExtended.type = std::string("image1d_t"); - break; - case NEOImageType::ImageType1DArray: - argMetadataExtended.type = std::string("image1d_array_t"); - break; - case NEOImageType::ImageType2DArray: - argMetadataExtended.type = std::string("image2d_array_t"); - break; - case NEOImageType::ImageType3D: - argMetadataExtended.type = std::string("image3d_t"); - break; - case NEOImageType::ImageType2DDepth: - argMetadataExtended.type = std::string("image2d_depth_t"); - break; - case NEOImageType::ImageType2DArrayDepth: - argMetadataExtended.type = std::string("image2d_array_depth_t"); - break; - case NEOImageType::ImageType2DMSAA: - argMetadataExtended.type = std::string("image2d_msaa_t"); - break; - case NEOImageType::ImageType2DMSAADepth: - argMetadataExtended.type = std::string("image2d_msaa_depth_t"); - break; - case NEOImageType::ImageType2DArrayMSAA: - argMetadataExtended.type = std::string("image2d_array_msaa_t"); - break; - case NEOImageType::ImageType2DArrayMSAADepth: - argMetadataExtended.type = std::string("image2d_array_msaa_depth_t"); - break; - default: - argMetadataExtended.type = std::string("image2d_t"); - break; - } - } else if (kernelArg.is()) { - argMetadataExtended.type = std::string("sampler_t"); - } - kernelInfo->kernelDescriptor.explicitArgsExtendedMetadata.at(argIndex) = std::move(argMetadataExtended); - argIndex++; - } - } - }; - std::call_once(generateDefaultMetadataOnce, generateDefaultMetadata); -} - } // namespace NEO diff --git a/opencl/source/program/program.h b/opencl/source/program/program.h index 976aa709d5..7b31ef97e8 100644 --- a/opencl/source/program/program.h +++ b/opencl/source/program/program.h @@ -282,7 +282,6 @@ class Program : public BaseObject<_cl_program> { MOCKABLE_VIRTUAL void createDebugZebin(uint32_t rootDeviceIndex); Debug::Segments getZebinSegments(uint32_t rootDeviceIndex); MOCKABLE_VIRTUAL void callPopulateZebinExtendedArgsMetadataOnce(uint32_t rootDeviceIndex); - MOCKABLE_VIRTUAL void callGenerateDefaultExtendedArgsMetadataOnce(uint32_t rootDeviceIndex); protected: MOCKABLE_VIRTUAL cl_int createProgramFromBinary(const void *pBinary, size_t binarySize, ClDevice &clDevice); @@ -377,7 +376,6 @@ class Program : public BaseObject<_cl_program> { size_t exportedFunctionsKernelId = std::numeric_limits::max(); std::once_flag extractAndDecodeMetadataOnce; - std::once_flag generateDefaultMetadataOnce; }; } // namespace NEO diff --git a/opencl/test/unit_test/kernel/kernel_arg_info_tests.cpp b/opencl/test/unit_test/kernel/kernel_arg_info_tests.cpp index 830f006f53..dffbd2834f 100644 --- a/opencl/test/unit_test/kernel/kernel_arg_info_tests.cpp +++ b/opencl/test/unit_test/kernel/kernel_arg_info_tests.cpp @@ -61,6 +61,12 @@ TEST_F(KernelArgInfoTest, GivenValidArgIndexThenExplicitArgsMetadataIsPopulated) EXPECT_TRUE(program->wasPopulateZebinExtendedArgsMetadataOnceCalled); } +TEST_F(KernelArgInfoTest, GivenEmptyExplicitArgsMetadataThenAppropriateErrorIsReturned) { + kernelDescriptor->explicitArgsExtendedMetadata.resize(0); + auto retVal = kernel->getArgInfo(0, 0, 0, nullptr, nullptr); + EXPECT_EQ(CL_KERNEL_ARG_INFO_NOT_AVAILABLE, retVal); +} + TEST_F(KernelArgInfoTest, GivenInvalidParametersWhenGettingKernelArgInfoThenValueSizeRetIsNotUpdated) { auto retVal = kernel->getArgInfo(0, 0, 0, nullptr, nullptr); EXPECT_EQ(CL_INVALID_VALUE, retVal); diff --git a/opencl/test/unit_test/mocks/mock_program.h b/opencl/test/unit_test/mocks/mock_program.h index 05f614c7d4..c7c894bd68 100644 --- a/opencl/test/unit_test/mocks/mock_program.h +++ b/opencl/test/unit_test/mocks/mock_program.h @@ -221,14 +221,6 @@ class MockProgram : public Program { } } - void callGenerateDefaultExtendedArgsMetadataOnce(uint32_t rootDeviceIndex) override { - wasGenerateDefaultMetadataOnceCalled = true; - - if (callBaseGenerateDefaultMetadataOnce) { - return Program::callGenerateDefaultExtendedArgsMetadataOnce(rootDeviceIndex); - } - } - std::vector externalFunctions; std::map processGenBinaryCalledPerRootDevice; std::map replaceDeviceBinaryCalledPerRootDevice; @@ -241,9 +233,6 @@ class MockProgram : public Program { bool wasDebuggerNotified = false; bool wasPopulateZebinExtendedArgsMetadataOnceCalled = false; bool callBasePopulateZebinExtendedArgsMetadataOnce = false; - - bool wasGenerateDefaultMetadataOnceCalled = false; - bool callBaseGenerateDefaultMetadataOnce = false; }; class MockProgramAppendKernelDebugOptions : public Program { diff --git a/opencl/test/unit_test/program/program_tests.cpp b/opencl/test/unit_test/program/program_tests.cpp index b9778e577d..61cd146455 100644 --- a/opencl/test/unit_test/program/program_tests.cpp +++ b/opencl/test/unit_test/program/program_tests.cpp @@ -3482,81 +3482,3 @@ kernels_misc_info: buildInfo.kernelInfoArray.clear(); buildInfo.unpackedDeviceBinary.release(); } - -TEST(ProgramGenerateDefaultArgsMetadataTests, givenNativeBinaryWhenCallingGenerateDefaultExtendedArgsMetadataThenGenerateMetadataForEachExplicitArgForEachKernel) { - MockClDevice device{new MockDevice()}; - MockProgram program(toClDeviceVector(device)); - program.callBaseGenerateDefaultMetadataOnce = true; - - const auto &rootDeviceIndex = device.getRootDeviceIndex(); - auto &buildInfo = program.buildInfos[rootDeviceIndex]; - - KernelInfo kernelInfo1, kernelInfo2; - kernelInfo1.kernelDescriptor.kernelMetadata.kernelName = "some_kernel"; - kernelInfo2.kernelDescriptor.kernelMetadata.kernelName = "another_kernel"; - buildInfo.kernelInfoArray.push_back(&kernelInfo1); - buildInfo.kernelInfoArray.push_back(&kernelInfo2); - - kernelInfo1.kernelDescriptor.payloadMappings.explicitArgs.resize(2); - kernelInfo1.kernelDescriptor.payloadMappings.explicitArgs.at(0).type = ArgDescriptor::ArgTPointer; - kernelInfo1.kernelDescriptor.payloadMappings.explicitArgs.at(1).type = ArgDescriptor::ArgTImage; - auto &img = kernelInfo1.kernelDescriptor.payloadMappings.explicitArgs.at(1).as(); - img.imageType = NEOImageType::ImageType2D; - - kernelInfo2.kernelDescriptor.payloadMappings.explicitArgs.resize(1); - kernelInfo2.kernelDescriptor.payloadMappings.explicitArgs.at(0).type = ArgDescriptor::ArgTSampler; - - program.callGenerateDefaultExtendedArgsMetadataOnce(rootDeviceIndex); - EXPECT_EQ(2u, kernelInfo1.kernelDescriptor.explicitArgsExtendedMetadata.size()); - EXPECT_EQ(1u, kernelInfo2.kernelDescriptor.explicitArgsExtendedMetadata.size()); - - const auto &argMetadata1 = kernelInfo1.kernelDescriptor.explicitArgsExtendedMetadata[0]; - EXPECT_STREQ("arg0", argMetadata1.argName.c_str()); - EXPECT_STREQ("void*", argMetadata1.type.c_str()); - - const auto &argMetadata2 = kernelInfo1.kernelDescriptor.explicitArgsExtendedMetadata[1]; - EXPECT_STREQ("arg1", argMetadata2.argName.c_str()); - EXPECT_STREQ("image2d_t", argMetadata2.type.c_str()); - - const auto &argMetadata3 = kernelInfo2.kernelDescriptor.explicitArgsExtendedMetadata[0]; - EXPECT_STREQ("arg0", argMetadata3.argName.c_str()); - EXPECT_STREQ("sampler_t", argMetadata3.type.c_str()); - - buildInfo.kernelInfoArray.clear(); - buildInfo.unpackedDeviceBinary.release(); -} - -TEST(ProgramGenerateDefaultArgsMetadataTests, whenGeneratingDefaultMetadataForArgByValueWithManyElementsThenGenerateProperTypeName) { - MockClDevice device{new MockDevice()}; - MockProgram program(toClDeviceVector(device)); - program.callBaseGenerateDefaultMetadataOnce = true; - - const auto &rootDeviceIndex = device.getRootDeviceIndex(); - auto &buildInfo = program.buildInfos[rootDeviceIndex]; - - KernelInfo kernelInfo; - kernelInfo.kernelDescriptor.kernelMetadata.kernelName = "some_kernel"; - buildInfo.kernelInfoArray.push_back(&kernelInfo); - - kernelInfo.kernelDescriptor.payloadMappings.explicitArgs.resize(1); - kernelInfo.kernelDescriptor.payloadMappings.explicitArgs.at(0).type = ArgDescriptor::ArgTValue; - auto &argAsVal = kernelInfo.kernelDescriptor.payloadMappings.explicitArgs.at(0).as(); - argAsVal.elements.resize(3u); - - argAsVal.elements[0].sourceOffset = 0u; - argAsVal.elements[0].size = 8u; - argAsVal.elements[1].sourceOffset = 8u; - argAsVal.elements[1].size = 8u; - argAsVal.elements[2].sourceOffset = 16u; - argAsVal.elements[2].size = 8u; - - program.callGenerateDefaultExtendedArgsMetadataOnce(rootDeviceIndex); - EXPECT_EQ(1u, kernelInfo.kernelDescriptor.explicitArgsExtendedMetadata.size()); - - const auto &argMetadata = kernelInfo.kernelDescriptor.explicitArgsExtendedMetadata[0]; - EXPECT_STREQ("arg0", argMetadata.argName.c_str()); - EXPECT_STREQ("char[24]", argMetadata.type.c_str()); - - buildInfo.kernelInfoArray.clear(); - buildInfo.unpackedDeviceBinary.release(); -} \ No newline at end of file diff --git a/shared/source/device_binary_format/zebin_decoder.cpp b/shared/source/device_binary_format/zebin_decoder.cpp index 34b9ca228a..f834743603 100644 --- a/shared/source/device_binary_format/zebin_decoder.cpp +++ b/shared/source/device_binary_format/zebin_decoder.cpp @@ -887,11 +887,7 @@ NEO::DecodeError populateArgDescriptor(const NEO::Elf::ZebinKernelMetadata::Type dst.payloadMappings.explicitArgs[src.argIndex].as(true); break; case NEO::Elf::ZebinKernelMetadata::Types::Kernel::PayloadArgument::AddressSpaceImage: { - auto &argAsImage = dst.payloadMappings.explicitArgs[src.argIndex].as(true); - if (src.imageType != NEO::Elf::ZebinKernelMetadata::Types::Kernel::PayloadArgument::ImageTypeMax) { - argAsImage.imageType = static_cast(src.imageType); - } - + dst.payloadMappings.explicitArgs[src.argIndex].as(true); auto &extendedInfo = dst.payloadMappings.explicitArgs[src.argIndex].getExtendedTypeInfo(); extendedInfo.isMediaImage = (src.imageType == NEO::Elf::ZebinKernelMetadata::Types::Kernel::PayloadArgument::ImageType::ImageType2DMedia); extendedInfo.isMediaBlockImage = (src.imageType == NEO::Elf::ZebinKernelMetadata::Types::Kernel::PayloadArgument::ImageType::ImageType2DMediaBlock); diff --git a/shared/source/kernel/kernel_arg_descriptor.h b/shared/source/kernel/kernel_arg_descriptor.h index 1c882e69b7..802cda422b 100644 --- a/shared/source/kernel/kernel_arg_descriptor.h +++ b/shared/source/kernel/kernel_arg_descriptor.h @@ -45,26 +45,6 @@ struct ArgDescPointer final { } }; -enum class NEOImageType : uint8_t { - ImageTypeUnknown, - ImageTypeBuffer, - ImageType1D, - ImageType1DArray, - ImageType2D, - ImageType2DArray, - ImageType3D, - ImageTypeCube, - ImageTypeCubeArray, - ImageType2DDepth, - ImageType2DArrayDepth, - ImageType2DMSAA, - ImageType2DMSAADepth, - ImageType2DArrayMSAA, - ImageType2DArrayMSAADepth, - ImageType2DMedia, - ImageType2DMediaBlock, -}; - struct ArgDescImage final { SurfaceStateHeapOffset bindful = undefined; // stateful with BTI CrossThreadDataOffset bindless = undefined; @@ -84,7 +64,6 @@ struct ArgDescImage final { CrossThreadDataOffset flatHeight = undefined; CrossThreadDataOffset flatPitch = undefined; } metadataPayload; - NEOImageType imageType; }; struct ArgDescSampler final { @@ -203,7 +182,7 @@ struct ArgDescriptor final { namespace { constexpr auto ArgSize = sizeof(ArgDescriptor); -static_assert(ArgSize <= 72, "Keep it small"); +static_assert(ArgSize <= 64, "Keep it small"); } // namespace template <>