From 4c3e6992088fc62e2f24465de52ed015494802f5 Mon Sep 17 00:00:00 2001 From: "Wlodarczyk, Bertrand" Date: Fri, 22 Mar 2024 13:12:52 +0100 Subject: [PATCH] Integration of reporting api to parser From it's inception the reporting api had separate parsing pipeline - different from the rest of translator. This fix integrate reporting api with the main parsing routines with translation. In addition to that it also fixes a problem of separated validation rules when it comes to reporting api. Now --report-spirv must also obey command line validation rules (required extensions) like other commands. --- include/LLVMSPIRVLib.h | 21 ++--- include/LLVMSPIRVOpts.h | 6 ++ lib/SPIRV/SPIRVReader.cpp | 98 +++++------------------- lib/SPIRV/libSPIRV/SPIRVEntry.h | 5 ++ lib/SPIRV/libSPIRV/SPIRVModule.cpp | 19 ++++- test/negative/spirv_report_bad_input.spt | 33 -------- test/spirv_report.spt | 2 +- tools/llvm-spirv/llvm-spirv.cpp | 32 ++++---- 8 files changed, 72 insertions(+), 144 deletions(-) delete mode 100644 test/negative/spirv_report_bad_input.spt diff --git a/include/LLVMSPIRVLib.h b/include/LLVMSPIRVLib.h index 48834d8be8..96ae28e75c 100644 --- a/include/LLVMSPIRVLib.h +++ b/include/LLVMSPIRVLib.h @@ -107,33 +107,34 @@ std::unique_ptr readSpirvModule(std::istream &IS, std::string &ErrMsg); struct SPIRVModuleReport { - SPIRV::VersionNumber Version; + uint32_t Version; uint32_t MemoryModel; uint32_t AddrModel; - std::vector Extensions; - std::vector ExtendedInstructionSets; - std::vector Capabilities; + llvm::SmallVector Extensions; + llvm::SmallVector ExtendedInstructionSets; + llvm::SmallVector Capabilities; }; /// \brief Partially load SPIR-V from the stream and decode only selected /// instructions that are needed to retrieve general information /// about the module. If this call fails, readSPIRVModule is /// expected to fail as well. /// \returns nullopt on failure. -std::optional getSpirvReport(std::istream &IS); -std::optional getSpirvReport(std::istream &IS, int &ErrCode); +void getSpirvReport(std::istream &IS, SPIRV::TranslatorOpts &Opts, + SPIRVModuleReport &Report); struct SPIRVModuleTextReport { std::string Version; std::string MemoryModel; std::string AddrModel; - std::vector Extensions; - std::vector ExtendedInstructionSets; - std::vector Capabilities; + llvm::SmallVector Extensions; + llvm::SmallVector ExtendedInstructionSets; + llvm::SmallVector Capabilities; }; /// \brief Create a human-readable form of the report returned by a call to /// getSpirvReport by decoding its binary fields. /// \returns String with the human-readable report. -SPIRVModuleTextReport formatSpirvReport(const SPIRVModuleReport &Report); +void formatSpirvReport(const SPIRVModuleReport &Report, + SPIRVModuleTextReport &TextReport); /// \brief Returns the message associated with the error code. /// \returns empty string if no known error code is found. diff --git a/include/LLVMSPIRVOpts.h b/include/LLVMSPIRVOpts.h index f5e301fb8b..25eb0e6a11 100644 --- a/include/LLVMSPIRVOpts.h +++ b/include/LLVMSPIRVOpts.h @@ -158,6 +158,10 @@ class TranslatorOpts { void setPreserveAuxData(bool ArgValue) { PreserveAuxData = ArgValue; } + void setIsReport(bool IsReport) { this->IsReport = IsReport; } + + bool isReport() { return IsReport; } + void setGenKernelArgNameMDEnabled(bool ArgNameMD) { GenKernelArgNameMD = ArgNameMD; } @@ -285,6 +289,8 @@ class TranslatorOpts { bool PreserveAuxData = false; BuiltinFormat SPIRVBuiltinFormat = BuiltinFormat::Function; + + bool IsReport = false; }; } // namespace SPIRV diff --git a/lib/SPIRV/SPIRVReader.cpp b/lib/SPIRV/SPIRVReader.cpp index dd724a7343..2bb2100444 100644 --- a/lib/SPIRV/SPIRVReader.cpp +++ b/lib/SPIRV/SPIRVReader.cpp @@ -4875,86 +4875,25 @@ Instruction *SPIRVToLLVM::transRelational(SPIRVInstruction *I, BasicBlock *BB) { return cast(Mutator.getMutated()); } -std::optional getSpirvReport(std::istream &IS) { - int IgnoreErrCode; - return getSpirvReport(IS, IgnoreErrCode); -} - -std::optional getSpirvReport(std::istream &IS, - int &ErrCode) { - SPIRVWord Word; - std::string Name; - std::unique_ptr BM(SPIRVModule::createSPIRVModule()); - SPIRVDecoder D(IS, *BM); - D >> Word; - if (Word != MagicNumber) { - ErrCode = SPIRVEC_InvalidMagicNumber; - return {}; - } - D >> Word; - if (!isSPIRVVersionKnown(Word)) { - ErrCode = SPIRVEC_InvalidVersionNumber; - return {}; - } - SPIRVModuleReport Report; - Report.Version = static_cast(Word); - // Skip: Generator’s magic number, Bound and Reserved word - D.ignore(3); - - bool IsReportGenCompleted = false, IsMemoryModelDefined = false; - while (!IS.bad() && !IsReportGenCompleted && D.getWordCountAndOpCode()) { - switch (D.OpCode) { - case OpCapability: - D >> Word; - Report.Capabilities.push_back(Word); - break; - case OpExtension: - Name.clear(); - D >> Name; - Report.Extensions.push_back(Name); - break; - case OpExtInstImport: - Name.clear(); - D >> Word >> Name; - Report.ExtendedInstructionSets.push_back(Name); - break; - case OpMemoryModel: - if (IsMemoryModelDefined) { - ErrCode = SPIRVEC_RepeatedMemoryModel; - return {}; - } - SPIRVAddressingModelKind AddrModel; - SPIRVMemoryModelKind MemoryModel; - D >> AddrModel >> MemoryModel; - if (!isValid(AddrModel)) { - ErrCode = SPIRVEC_InvalidAddressingModel; - return {}; - } - if (!isValid(MemoryModel)) { - ErrCode = SPIRVEC_InvalidMemoryModel; - return {}; - } - Report.MemoryModel = MemoryModel; - Report.AddrModel = AddrModel; - IsMemoryModelDefined = true; - // In this report we don't analyze instructions after OpMemoryModel - IsReportGenCompleted = true; - break; - default: - // No more instructions to gather information about - IsReportGenCompleted = true; - } +void getSpirvReport(std::istream &IS, SPIRV::TranslatorOpts &Opts, + SPIRVModuleReport &Report) { + std::unique_ptr BM(SPIRVModule::createSPIRVModule(Opts)); + IS >> *BM; + auto &CM = BM->getCapability(); + for (auto &KV : CM) { + Report.Capabilities.emplace_back(KV.second->getKind()); } - if (IS.bad()) { - ErrCode = SPIRVEC_InvalidModule; - return {}; + auto &EM = BM->getExtension(); + for (auto &K : EM) { + Report.Extensions.emplace_back(K); } - if (!IsMemoryModelDefined) { - ErrCode = SPIRVEC_UnspecifiedMemoryModel; - return {}; + auto &SEM = BM->getSourceExtension(); + for (auto &SE : SEM) { + Report.ExtendedInstructionSets.emplace_back(SE); } - ErrCode = SPIRVEC_Success; - return std::make_optional(std::move(Report)); + Report.MemoryModel = BM->getMemoryModel(); + Report.Version = BM->getSPIRVVersion(); + Report.AddrModel = BM->getAddressingModel(); } constexpr std::string_view formatAddressingModel(uint32_t AddrModel) { @@ -4987,8 +4926,8 @@ constexpr std::string_view formatMemoryModel(uint32_t MemoryModel) { } } -SPIRVModuleTextReport formatSpirvReport(const SPIRVModuleReport &Report) { - SPIRVModuleTextReport TextReport; +void formatSpirvReport(const SPIRVModuleReport &Report, + SPIRVModuleTextReport &TextReport) { TextReport.Version = formatVersionNumber(static_cast(Report.Version)); TextReport.AddrModel = formatAddressingModel(Report.AddrModel); @@ -5003,7 +4942,6 @@ SPIRVModuleTextReport formatSpirvReport(const SPIRVModuleReport &Report) { // other fields with string content can be copied as is TextReport.Extensions = Report.Extensions; TextReport.ExtendedInstructionSets = Report.ExtendedInstructionSets; - return TextReport; } std::unique_ptr readSpirvModule(std::istream &IS, diff --git a/lib/SPIRV/libSPIRV/SPIRVEntry.h b/lib/SPIRV/libSPIRV/SPIRVEntry.h index c87aa21ec0..319adf719e 100644 --- a/lib/SPIRV/libSPIRV/SPIRVEntry.h +++ b/lib/SPIRV/libSPIRV/SPIRVEntry.h @@ -52,6 +52,7 @@ #include #include #include +#include #include namespace SPIRV { @@ -821,6 +822,8 @@ class SPIRVExtInstImport : public SPIRVEntry { // Incomplete constructor SPIRVExtInstImport() : SPIRVEntry(OC) {} + std::string_view getImportLiteral() { return Str; } + protected: _SPIRV_DCL_ENCDEC void validate() const override; @@ -911,6 +914,8 @@ class SPIRVCapability : public SPIRVEntryNoId { } } + SPIRVCapabilityKind getKind() { return Kind; } + private: SPIRVCapabilityKind Kind; }; diff --git a/lib/SPIRV/libSPIRV/SPIRVModule.cpp b/lib/SPIRV/libSPIRV/SPIRVModule.cpp index 0d0101e811..16d57b80ac 100644 --- a/lib/SPIRV/libSPIRV/SPIRVModule.cpp +++ b/lib/SPIRV/libSPIRV/SPIRVModule.cpp @@ -575,7 +575,7 @@ class SPIRVModuleImpl : public SPIRVModule { void layoutEntry(SPIRVEntry *Entry); std::istream &parseSPT(std::istream &I); - std::istream &parseSPIRV(std::istream &I); + template std::istream &parseSPIRV(std::istream &I); }; SPIRVModuleImpl::~SPIRVModuleImpl() { @@ -787,6 +787,12 @@ void SPIRVModuleImpl::layoutEntry(SPIRVEntry *E) { addTo(AsmVec, E); break; } + case OpExtInstImport: { + auto *EII = static_cast(E); + std::string_view IL = EII->getImportLiteral(); + SrcExtension.emplace(IL); + break; + } default: if (isTypeOpCode(OC)) TypeVec.push_back(static_cast(E)); @@ -2321,6 +2327,7 @@ std::istream &SPIRVModuleImpl::parseSPT(std::istream &I) { return I; } +template std::istream &SPIRVModuleImpl::parseSPIRV(std::istream &I) { SPIRVModuleImpl &MI = *this; MI.setAutoAddCapability(false); @@ -2387,6 +2394,11 @@ std::istream &SPIRVModuleImpl::parseSPIRV(std::istream &I) { SPIRVDBG(spvdbgs() << "getWordCountAndOpCode EOF 0 0\n"); break; } + if constexpr (IS_REPORT) { + if (OpCode == OpMemoryModel) { + break; + } + } } MI.resolveUnknownStructFields(); return I; @@ -2399,7 +2411,10 @@ std::istream &operator>>(std::istream &I, SPIRVModule &M) { return MI.parseSPT(I); } #endif - return MI.parseSPIRV(I); + if (!MI.TranslationOpts.isReport()) { + return MI.parseSPIRV(I); + } + return MI.parseSPIRV(I); } SPIRVModule *SPIRVModule::createSPIRVModule() { return new SPIRVModuleImpl(); } diff --git a/test/negative/spirv_report_bad_input.spt b/test/negative/spirv_report_bad_input.spt deleted file mode 100644 index a2c8cd9382..0000000000 --- a/test/negative/spirv_report_bad_input.spt +++ /dev/null @@ -1,33 +0,0 @@ -; RUN: llvm-spirv %s -to-binary -o %t.spv -; The next line is to corrupt the binary file by changing its Magic Number -; RUN: echo "0" > %t_corrupted.spv && cat %t.spv >> %t_corrupted.spv -; RUN: not llvm-spirv --spirv-print-report %t_corrupted.spv 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR -; -; CHECK-ERROR: Invalid SPIR-V binary: "InvalidMagicNumber: Invalid Magic Number." - -119734787 65536 393230 10 0 -2 Capability Addresses -2 Capability Kernel -2 Capability LoopFuseINTEL -2 Capability BitInstructions -6 Extension "SPV_INTEL_loop_fuse" -8 Extension "SPV_KHR_bit_instructions" -5 ExtInstImport 1 "OpenCL.std" -3 MemoryModel 1 2 -7 EntryPoint 6 5 "TestSatPacked" -3 Source 3 102000 - -5 Decorate 5 FuseLoopsInFunctionINTEL 3 1 -4 TypeInt 3 32 0 -2 TypeVoid 2 -5 TypeFunction 4 2 3 3 - -5 Function 2 5 0 4 -3 FunctionParameter 3 6 -3 FunctionParameter 3 7 - -2 Label 8 -4 BitReverse 3 9 6 -1 Return - -1 FunctionEnd diff --git a/test/spirv_report.spt b/test/spirv_report.spt index a8d89abee3..38cd300036 100644 --- a/test/spirv_report.spt +++ b/test/spirv_report.spt @@ -1,5 +1,5 @@ ; RUN: llvm-spirv %s -to-binary -o %t.spv -; RUN: llvm-spirv --spirv-print-report %t.spv | FileCheck %s --check-prefix=CHECK-DAG +; RUN: llvm-spirv --spirv-print-report --spirv-ext=+SPV_INTEL_loop_fuse,+SPV_KHR_bit_instructions %t.spv | FileCheck %s --check-prefix=CHECK-DAG ; CHECK-DAG: Version: 1.0 ; CHECK-DAG: Memory model: OpenCL diff --git a/tools/llvm-spirv/llvm-spirv.cpp b/tools/llvm-spirv/llvm-spirv.cpp index 35a7c4d7c0..fe259b8ef6 100644 --- a/tools/llvm-spirv/llvm-spirv.cpp +++ b/tools/llvm-spirv/llvm-spirv.cpp @@ -864,36 +864,32 @@ int main(int Ac, char **Av) { if (SPIRVPrintReport) { std::ifstream IFS(InputFile, std::ios::binary); - int ErrCode = 0; - std::optional BinReport = - SPIRV::getSpirvReport(IFS, ErrCode); - if (!BinReport) { - std::cerr << "Invalid SPIR-V binary: \"" << SPIRV::getErrorMessage(ErrCode) << "\"\n"; - return -1; - } + Opts.setIsReport(true); + SPIRV::SPIRVModuleReport BinReport{}; + SPIRV::SPIRVModuleTextReport TextReport{}; + SPIRV::getSpirvReport(IFS, Opts, BinReport); + SPIRV::formatSpirvReport(BinReport, TextReport); - SPIRV::SPIRVModuleTextReport TextReport = - SPIRV::formatSpirvReport(BinReport.value()); - - std::cout << "SPIR-V module report:" - << "\n Version: " << TextReport.Version + std::cout << "SPIR-V module report:" << "\n Version: " << TextReport.Version << "\n Memory model: " << TextReport.MemoryModel << "\n Addressing model: " << TextReport.AddrModel << "\n"; std::cout << " Number of capabilities: " << TextReport.Capabilities.size() << "\n"; - for (auto &Capability : TextReport.Capabilities) + for (auto &Capability : TextReport.Capabilities) { std::cout << " Capability: " << Capability << "\n"; - + } std::cout << " Number of extensions: " << TextReport.Extensions.size() << "\n"; - for (auto &Extension : TextReport.Extensions) + for (auto &Extension : TextReport.Extensions) { std::cout << " Extension: " << Extension << "\n"; - + } std::cout << " Number of extended instruction sets: " << TextReport.ExtendedInstructionSets.size() << "\n"; - for (auto &ExtendedInstructionSet : TextReport.ExtendedInstructionSets) - std::cout << " Extended Instruction Set: " << ExtendedInstructionSet << "\n"; + for (auto &ExtendedInstructionSet : TextReport.ExtendedInstructionSets) { + std::cout << " Extended Instruction Set: " << ExtendedInstructionSet + << "\n"; + } } return 0; }