Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Integration of reporting api with translator parser #2457

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Integration of reporting api with translator parser #2457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions include/LLVMSPIRVLib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,33 +107,34 @@ std::unique_ptr<SPIRVModule> readSpirvModule(std::istream &IS,
std::string &ErrMsg);

struct SPIRVModuleReport {
SPIRV::VersionNumber Version;
uint32_t Version;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably shouldn't change a type for the Version

uint32_t MemoryModel;
uint32_t AddrModel;
std::vector<std::string> Extensions;
std::vector<std::string> ExtendedInstructionSets;
std::vector<uint32_t> Capabilities;
llvm::SmallVector<std::string> Extensions;
llvm::SmallVector<std::string> ExtendedInstructionSets;
llvm::SmallVector<uint32_t> 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<SPIRVModuleReport> getSpirvReport(std::istream &IS);
std::optional<SPIRVModuleReport> 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<std::string> Extensions;
std::vector<std::string> ExtendedInstructionSets;
std::vector<std::string> Capabilities;
llvm::SmallVector<std::string> Extensions;
llvm::SmallVector<std::string> ExtendedInstructionSets;
llvm::SmallVector<std::string> 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.
Expand Down
6 changes: 6 additions & 0 deletions include/LLVMSPIRVOpts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -285,6 +289,8 @@ class TranslatorOpts {
bool PreserveAuxData = false;

BuiltinFormat SPIRVBuiltinFormat = BuiltinFormat::Function;

bool IsReport = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a small comment about this field. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field only purpose is to go around iostream interface to make branch in parser routine and it won't last. Should be removed during removal of iostream implementation.

};

} // namespace SPIRV
Expand Down
98 changes: 18 additions & 80 deletions lib/SPIRV/SPIRVReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4875,86 +4875,25 @@ Instruction *SPIRVToLLVM::transRelational(SPIRVInstruction *I, BasicBlock *BB) {
return cast<Instruction>(Mutator.getMutated());
}

std::optional<SPIRVModuleReport> getSpirvReport(std::istream &IS) {
int IgnoreErrCode;
return getSpirvReport(IS, IgnoreErrCode);
}

std::optional<SPIRVModuleReport> getSpirvReport(std::istream &IS,
int &ErrCode) {
SPIRVWord Word;
std::string Name;
std::unique_ptr<SPIRVModule> 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<SPIRV::VersionNumber>(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<SPIRVModule> 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) {
Expand Down Expand Up @@ -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<uint32_t>(Report.Version));
TextReport.AddrModel = formatAddressingModel(Report.AddrModel);
Expand All @@ -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<SPIRVModule> readSpirvModule(std::istream &IS,
Expand Down
5 changes: 5 additions & 0 deletions lib/SPIRV/libSPIRV/SPIRVEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include <optional>
#include <set>
#include <string>
#include <string_view>
#include <vector>

namespace SPIRV {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -911,6 +914,8 @@ class SPIRVCapability : public SPIRVEntryNoId<OpCapability> {
}
}

SPIRVCapabilityKind getKind() { return Kind; }

private:
SPIRVCapabilityKind Kind;
};
Expand Down
19 changes: 17 additions & 2 deletions lib/SPIRV/libSPIRV/SPIRVModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <bool IS_REPORT> std::istream &parseSPIRV(std::istream &I);
};

SPIRVModuleImpl::~SPIRVModuleImpl() {
Expand Down Expand Up @@ -787,6 +787,12 @@ void SPIRVModuleImpl::layoutEntry(SPIRVEntry *E) {
addTo(AsmVec, E);
break;
}
case OpExtInstImport: {
auto *EII = static_cast<SPIRVExtInstImport *>(E);
std::string_view IL = EII->getImportLiteral();
SrcExtension.emplace(IL);
break;
}
default:
if (isTypeOpCode(OC))
TypeVec.push_back(static_cast<SPIRVType *>(E));
Expand Down Expand Up @@ -2321,6 +2327,7 @@ std::istream &SPIRVModuleImpl::parseSPT(std::istream &I) {
return I;
}

template <bool IS_REPORT>
std::istream &SPIRVModuleImpl::parseSPIRV(std::istream &I) {
SPIRVModuleImpl &MI = *this;
MI.setAutoAddCapability(false);
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does look fishy, why do we need to skip OpMemoryModel? There should be no more then 2 OpMemoryModel in the model.

break;
}
}
}
MI.resolveUnknownStructFields();
return I;
Expand All @@ -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<false>(I);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not pass the isReport as an argument? Templated code might result in unneeded code bloating here.

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, at this stage no. We don't want to introduce unneeded branch in parser hot path for both report and binary cases. User using binary translation shouldn't pay price for Reporting API. This could be avoided if the implementation wouldn't be in iostreams. That makes efficient implementation of early stop very hard.
In this case although you have right that the code size will increase due to two implementations of the same function but prom performance PoV shouldn't make any difference. During first enter icache would be cold anyways and then it's circulating in the same hot routines. There won't be a situation when you have both <true> and <false> cases loaded to icache in the same time so really problem is negligible. What's more this template can be removed during removal of iostreams as main parsing system.

}
return MI.parseSPIRV<true>(I);
}

SPIRVModule *SPIRVModule::createSPIRVModule() { return new SPIRVModuleImpl(); }
Expand Down
33 changes: 0 additions & 33 deletions test/negative/spirv_report_bad_input.spt
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion test/spirv_report.spt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be reported if an extension is missing in spirv-ext? Can we add a CHECK for that?

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly the same like in other tests which covers situation with missing extensions. We already have such tests. For more detailed response please look on the answer below.

; 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
Expand Down
32 changes: 14 additions & 18 deletions tools/llvm-spirv/llvm-spirv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,36 +864,32 @@ int main(int Ac, char **Av) {

if (SPIRVPrintReport) {
std::ifstream IFS(InputFile, std::ios::binary);
int ErrCode = 0;
std::optional<SPIRV::SPIRVModuleReport> 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;
}
Loading