From aedcf7a09f21342b9f27e265e7ed2cc34777bf3d Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Wed, 31 Jan 2024 17:15:25 -0800 Subject: [PATCH 1/2] [NFCI][SYCL] Refactor getBinaryImageFormat A future PR will add support for magic numbers other than four bytes. Refactor the code to make those future changes easier to review. --- sycl/source/detail/pi.cpp | 72 ++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/sycl/source/detail/pi.cpp b/sycl/source/detail/pi.cpp index 92bccc2cea2b2..9d343bf8eac6d 100644 --- a/sycl/source/detail/pi.cpp +++ b/sycl/source/detail/pi.cpp @@ -685,45 +685,41 @@ static uint16_t getELFHeaderType(const unsigned char *ImgData, size_t ImgSize) { sycl::detail::pi::PiDeviceBinaryType getBinaryImageFormat(const unsigned char *ImgData, size_t ImgSize) { // Top-level magic numbers for the recognized binary image formats. - struct { - sycl::detail::pi::PiDeviceBinaryType Fmt; - const uint32_t Magic; - } Fmts[] = {{PI_DEVICE_BINARY_TYPE_SPIRV, 0x07230203}, - {PI_DEVICE_BINARY_TYPE_LLVMIR_BITCODE, 0xDEC04342}, - // 'I', 'N', 'T', 'C' ; Intel native - {PI_DEVICE_BINARY_TYPE_NATIVE, 0x43544E49}}; - - if (ImgSize >= sizeof(Fmts[0].Magic)) { - std::remove_const_t Hdr = 0; - std::copy(ImgData, ImgData + sizeof(Hdr), reinterpret_cast(&Hdr)); - - // Check headers for direct formats. - for (const auto &Fmt : Fmts) { - if (Hdr == Fmt.Magic) - return Fmt.Fmt; - } + auto MatchMagicNumber = [&](auto Number) { + if (ImgSize < sizeof(Number)) + return false; + return std::memcmp(ImgData, &Number, sizeof(Number)) == 0; + }; + + if (MatchMagicNumber(uint32_t{0x07230203})) + return PI_DEVICE_BINARY_TYPE_SPIRV; + + if (MatchMagicNumber(uint32_t{0xDEC04342})) + return PI_DEVICE_BINARY_TYPE_LLVMIR_BITCODE; + + if (MatchMagicNumber(uint32_t{0x43544E49})) + // 'I', 'N', 'T', 'C' ; Intel native + return PI_DEVICE_BINARY_TYPE_LLVMIR_BITCODE; + + // Check for ELF format, size requirements include data we'll read in case of + // succesful match. + if (ImgSize < 18 || !MatchMagicNumber(uint32_t{0x464c457F})) + return PI_DEVICE_BINARY_TYPE_NONE; + + uint16_t ELFHdrType = getELFHeaderType(ImgData, ImgSize); + if (ELFHdrType == 0xFF04) + // OpenCL executable. + return PI_DEVICE_BINARY_TYPE_NATIVE; + + if (ELFHdrType == 0xFF12) + // ZEBIN executable. + return PI_DEVICE_BINARY_TYPE_NATIVE; + + // Newer ZEBIN format does not have a special header type, but can instead + // be identified by having a required .ze_info section. + if (checkELFSectionPresent(".ze_info", ImgData, ImgSize)) + return PI_DEVICE_BINARY_TYPE_NATIVE; - // ELF e_type for recognized binary image formats. - struct { - sycl::detail::pi::PiDeviceBinaryType Fmt; - const uint16_t Magic; - } ELFFmts[] = {{PI_DEVICE_BINARY_TYPE_NATIVE, 0xFF04}, // OpenCL executable - {PI_DEVICE_BINARY_TYPE_NATIVE, 0xFF12}}; // ZEBIN executable - - // ELF files need to be parsed separately. The header type ends after 18 - // bytes. - if (Hdr == 0x464c457F && ImgSize >= 18) { - uint16_t HdrType = getELFHeaderType(ImgData, ImgSize); - for (const auto &ELFFmt : ELFFmts) { - if (HdrType == ELFFmt.Magic) - return ELFFmt.Fmt; - } - // Newer ZEBIN format does not have a special header type, but can instead - // be identified by having a required .ze_info section. - if (checkELFSectionPresent(".ze_info", ImgData, ImgSize)) - return PI_DEVICE_BINARY_TYPE_NATIVE; - } - } return PI_DEVICE_BINARY_TYPE_NONE; } From f017f4adad266a008bfd2a334a32525acc28a74a Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Fri, 2 Feb 2024 08:54:00 -0800 Subject: [PATCH 2/2] Address code-review comments --- sycl/source/detail/pi.cpp | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/sycl/source/detail/pi.cpp b/sycl/source/detail/pi.cpp index 9d343bf8eac6d..4dbeaccc9baf2 100644 --- a/sycl/source/detail/pi.cpp +++ b/sycl/source/detail/pi.cpp @@ -686,9 +686,8 @@ sycl::detail::pi::PiDeviceBinaryType getBinaryImageFormat(const unsigned char *ImgData, size_t ImgSize) { // Top-level magic numbers for the recognized binary image formats. auto MatchMagicNumber = [&](auto Number) { - if (ImgSize < sizeof(Number)) - return false; - return std::memcmp(ImgData, &Number, sizeof(Number)) == 0; + return ImgSize >= sizeof(Number) && + std::memcmp(ImgData, &Number, sizeof(Number)) == 0; }; if (MatchMagicNumber(uint32_t{0x07230203})) @@ -703,22 +702,21 @@ getBinaryImageFormat(const unsigned char *ImgData, size_t ImgSize) { // Check for ELF format, size requirements include data we'll read in case of // succesful match. - if (ImgSize < 18 || !MatchMagicNumber(uint32_t{0x464c457F})) - return PI_DEVICE_BINARY_TYPE_NONE; - - uint16_t ELFHdrType = getELFHeaderType(ImgData, ImgSize); - if (ELFHdrType == 0xFF04) - // OpenCL executable. - return PI_DEVICE_BINARY_TYPE_NATIVE; - - if (ELFHdrType == 0xFF12) - // ZEBIN executable. - return PI_DEVICE_BINARY_TYPE_NATIVE; - - // Newer ZEBIN format does not have a special header type, but can instead - // be identified by having a required .ze_info section. - if (checkELFSectionPresent(".ze_info", ImgData, ImgSize)) - return PI_DEVICE_BINARY_TYPE_NATIVE; + if (ImgSize >= 18 && MatchMagicNumber(uint32_t{0x464c457F})) { + uint16_t ELFHdrType = getELFHeaderType(ImgData, ImgSize); + if (ELFHdrType == 0xFF04) + // OpenCL executable. + return PI_DEVICE_BINARY_TYPE_NATIVE; + + if (ELFHdrType == 0xFF12) + // ZEBIN executable. + return PI_DEVICE_BINARY_TYPE_NATIVE; + + // Newer ZEBIN format does not have a special header type, but can instead + // be identified by having a required .ze_info section. + if (checkELFSectionPresent(".ze_info", ImgData, ImgSize)) + return PI_DEVICE_BINARY_TYPE_NATIVE; + } return PI_DEVICE_BINARY_TYPE_NONE; }