Skip to content

Commit

Permalink
Merge (conflict) 7b7a193 Cherry pick a couple of commits to improve l…
Browse files Browse the repository at this point in the history 
…lvm-spirv perf.. '"xmain"' -> '"xmain-cand"' (1 commits)

  CONFLICT (content): Merge conflict in test/transcoding/annotate_attribute.ll
  CONFLICT (content): Merge conflict in test/transcoding/NoSignedUnsignedWrap.ll

  commit 7b7a193
  Author: Alexey Sotkin <[email protected]>
  Date:   Tue Oct 26 11:12:43 2021 +0300

      Cherry pick a couple of commits to improve llvm-spirv performance (KhronosGroup#46)

      * [NFC] Optimize adding of decorations (KhronosGroup#1233)

      Use std::undordered_set instead of std::multiset with custom comparator.
      It is a bit unclear why std::multiset was required since there is no
      order requirement for OpDecorate instructions in SPIR-V spec, in addition old
      data structure was highly inefficient.

      * [NFC] Optimize OCLUtil::toString usage in checkError function (KhronosGroup#1229)

      `toString` function is called to convert an LLVM IR instruction to a string,
      mostly in order to include it into an error message in case LLVM IR is invalid.
      The problem is that this function is called even when IR is valid which
      significantly slows translator's work.

      This patch reworks `toString` usage in `checkError` method to generate a string
      only in case when LLVM IR is invalid.

      Co-authored-by: Mariya Podchishchaeva <[email protected]>
      Co-authored-by: Viktoria Maximova <[email protected]>
  • Loading branch information
iclsrc committed Oct 26, 2021
2 parents e84902e + 7b7a193 commit 2b19c10
Show file tree
Hide file tree
Showing 32 changed files with 250 additions and 292 deletions.
8 changes: 0 additions & 8 deletions lib/SPIRV/OCLUtil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,14 +494,6 @@ bool isSamplerTy(Type *Ty);
// Checks if the binary operator is an unfused fmul + fadd instruction.
bool isUnfusedMulAdd(BinaryOperator *B);

template <typename T> std::string toString(const T *Object) {
std::string S;
llvm::raw_string_ostream RSOS(S);
Object->print(RSOS);
RSOS.flush();
return S;
}

// Get data and vector size postfix for sugroup_block_{read|write} builtins
// as specified by cl_intel_subgroups* extensions.
// Scalar data assumed to be represented as vector of one element.
Expand Down
63 changes: 29 additions & 34 deletions lib/SPIRV/SPIRVWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1034,9 +1034,8 @@ SPIRV::SPIRVInstruction *LLVMToSPIRVBase::transUnaryInst(UnaryInstruction *U,
const auto DestAddrSpace = Cast->getDestTy()->getPointerAddressSpace();
if (DestAddrSpace == SPIRAS_Generic) {
getErrorLog().checkError(
SrcAddrSpace != SPIRAS_Constant, SPIRVEC_InvalidModule,
"Casts from constant address space to generic are illegal\n" +
toString(U));
SrcAddrSpace != SPIRAS_Constant, SPIRVEC_InvalidModule, U,
"Casts from constant address space to generic are illegal\n");
BOC = OpPtrCastToGeneric;
// In SPIR-V only casts to/from generic are allowed. But with
// SPV_INTEL_usm_storage_classes we can also have casts from global_device
Expand All @@ -1045,10 +1044,9 @@ SPIRV::SPIRVInstruction *LLVMToSPIRVBase::transUnaryInst(UnaryInstruction *U,
SrcAddrSpace == SPIRAS_GlobalHost) {
getErrorLog().checkError(DestAddrSpace == SPIRAS_Global ||
DestAddrSpace == SPIRAS_Generic,
SPIRVEC_InvalidModule,
SPIRVEC_InvalidModule, U,
"Casts from global_device/global_host only "
"allowed to global/generic\n" +
toString(U));
"allowed to global/generic\n");
if (!BM->isAllowedToUseExtension(
ExtensionID::SPV_INTEL_usm_storage_classes)) {
if (DestAddrSpace == SPIRAS_Global)
Expand All @@ -1061,10 +1059,9 @@ SPIRV::SPIRVInstruction *LLVMToSPIRVBase::transUnaryInst(UnaryInstruction *U,
DestAddrSpace == SPIRAS_GlobalHost) {
getErrorLog().checkError(SrcAddrSpace == SPIRAS_Global ||
SrcAddrSpace == SPIRAS_Generic,
SPIRVEC_InvalidModule,
SPIRVEC_InvalidModule, U,
"Casts to global_device/global_host only "
"allowed from global/generic\n" +
toString(U));
"allowed from global/generic\n");
if (!BM->isAllowedToUseExtension(
ExtensionID::SPV_INTEL_usm_storage_classes)) {
if (SrcAddrSpace == SPIRAS_Global)
Expand All @@ -1075,14 +1072,12 @@ SPIRV::SPIRVInstruction *LLVMToSPIRVBase::transUnaryInst(UnaryInstruction *U,
}
} else {
getErrorLog().checkError(
SrcAddrSpace == SPIRAS_Generic, SPIRVEC_InvalidModule,
SrcAddrSpace == SPIRAS_Generic, SPIRVEC_InvalidModule, U,
"Casts from private/local/global address space are allowed only to "
"generic\n" +
toString(U));
"generic\n");
getErrorLog().checkError(
DestAddrSpace != SPIRAS_Constant, SPIRVEC_InvalidModule,
"Casts from generic address space to constant are illegal\n" +
toString(U));
DestAddrSpace != SPIRAS_Constant, SPIRVEC_InvalidModule, U,
"Casts from generic address space to constant are illegal\n");
BOC = OpGenericCastToPtr;
}
} else {
Expand Down Expand Up @@ -1898,9 +1893,8 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
AtomicRMWInst::BinOp Op = ARMW->getOperation();
if (!BM->getErrorLog().checkError(
!AtomicRMWInst::isFPOperation(Op) && Op != AtomicRMWInst::Nand,
SPIRVEC_InvalidInstruction,
OCLUtil::toString(V) + "\nAtomic " +
AtomicRMWInst::getOperationName(Op).str() +
SPIRVEC_InvalidInstruction, V,
"Atomic " + AtomicRMWInst::getOperationName(Op).str() +
" is not supported in SPIR-V!\n"))
return nullptr;

Expand Down Expand Up @@ -3217,10 +3211,10 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II,
return BM->addInstTemplate(OpSaveMemoryINTEL, BB, Ty);
}
BM->getErrorLog().checkError(
BM->isUnknownIntrinsicAllowed(II), SPIRVEC_InvalidFunctionCall,
toString(II) + "\nTranslation of llvm.stacksave intrinsic requires "
"SPV_INTEL_variable_length_array extension or "
"-spirv-allow-unknown-intrinsics option.");
BM->isUnknownIntrinsicAllowed(II), SPIRVEC_InvalidFunctionCall, II,
"Translation of llvm.stacksave intrinsic requires "
"SPV_INTEL_variable_length_array extension or "
"-spirv-allow-unknown-intrinsics option.");
break;
}
case Intrinsic::stackrestore: {
Expand All @@ -3231,10 +3225,10 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II,
nullptr);
}
BM->getErrorLog().checkError(
BM->isUnknownIntrinsicAllowed(II), SPIRVEC_InvalidFunctionCall,
toString(II) + "\nTranslation of llvm.restore intrinsic requires "
"SPV_INTEL_variable_length_array extension or "
"-spirv-allow-unknown-intrinsics option.");
BM->isUnknownIntrinsicAllowed(II), SPIRVEC_InvalidFunctionCall, II,
"Translation of llvm.restore intrinsic requires "
"SPV_INTEL_variable_length_array extension or "
"-spirv-allow-unknown-intrinsics option.");
break;
}
// We can just ignore/drop some intrinsics, like optimizations hint.
Expand Down Expand Up @@ -3421,14 +3415,15 @@ SPIRVValue *LLVMToSPIRVBase::transDirectCallInst(CallInst *CI,

SPIRVValue *LLVMToSPIRVBase::transIndirectCallInst(CallInst *CI,
SPIRVBasicBlock *BB) {
if (!BM->checkExtension(ExtensionID::SPV_INTEL_function_pointers,
SPIRVEC_FunctionPointers, toString(CI)))
return nullptr;

return BM->addIndirectCallInst(
transValue(CI->getCalledOperand(), BB), transType(CI->getType()),
transArguments(CI, BB, SPIRVEntry::createUnique(OpFunctionCall).get()),
BB);
if (BM->getErrorLog().checkError(
BM->isAllowedToUseExtension(ExtensionID::SPV_INTEL_function_pointers),
SPIRVEC_FunctionPointers, CI)) {
return BM->addIndirectCallInst(
transValue(CI->getCalledOperand(), BB), transType(CI->getType()),
transArguments(CI, BB, SPIRVEntry::createUnique(OpFunctionCall).get()),
BB);
}
return nullptr;
}

SPIRVValue *LLVMToSPIRVBase::transAsmINTEL(InlineAsm *IA) {
Expand Down
57 changes: 2 additions & 55 deletions lib/SPIRV/libSPIRV/SPIRVDecorate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
#include "SPIRVValue.h"

namespace SPIRV {
template <class T, class B>
spv_ostream &operator<<(spv_ostream &O, const std::multiset<T *, B> &V) {
template <class T>
spv_ostream &operator<<(spv_ostream &O, const std::vector<T *> &V) {
for (auto &I : V)
O << *I;
return O;
Expand Down Expand Up @@ -286,57 +286,4 @@ void SPIRVGroupMemberDecorate::decorateTargets() {
}
}
}

bool SPIRVDecorateGeneric::Comparator::
operator()(const SPIRVDecorateGeneric *A, const SPIRVDecorateGeneric *B) const {
auto Action = [=]() {
if (A->getOpCode() < B->getOpCode())
return true;
if (A->getOpCode() > B->getOpCode())
return false;
if (A->getDecorateKind() < B->getDecorateKind())
return true;
if (A->getDecorateKind() > B->getDecorateKind())
return false;
if (A->getLiteralCount() < B->getLiteralCount())
return true;
if (A->getLiteralCount() > B->getLiteralCount())
return false;
for (size_t I = 0, E = A->getLiteralCount(); I != E; ++I) {
auto EA = A->getLiteral(I);
auto EB = B->getLiteral(I);
if (EA < EB)
return true;
if (EA > EB)
return false;
}
return false;
};
auto Res = Action();
return Res;
}

bool operator==(const SPIRVDecorateGeneric &A, const SPIRVDecorateGeneric &B) {
if (A.getTargetId() != B.getTargetId())
return false;
if (A.getOpCode() != B.getOpCode())
return false;
if (B.isMemberDecorate()) {
auto &MDA = static_cast<SPIRVMemberDecorate const &>(A);
auto &MDB = static_cast<SPIRVMemberDecorate const &>(B);
if (MDA.getMemberNumber() != MDB.getMemberNumber())
return false;
}
if (A.getDecorateKind() != B.getDecorateKind())
return false;
if (A.getLiteralCount() != B.getLiteralCount())
return false;
for (size_t I = 0, E = A.getLiteralCount(); I != E; ++I) {
auto EA = A.getLiteral(I);
auto EB = B.getLiteral(I);
if (EA != EB)
return false;
}
return true;
}
} // namespace SPIRV
36 changes: 4 additions & 32 deletions lib/SPIRV/libSPIRV/SPIRVDecorate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,6 @@ class SPIRVDecorateGeneric : public SPIRVAnnotationGeneric {
std::vector<SPIRVWord> getVecLiteral() const;
Decoration getDecorateKind() const;
size_t getLiteralCount() const;
/// Compare for kind and literal only.
struct Comparator {
bool operator()(const SPIRVDecorateGeneric *A,
const SPIRVDecorateGeneric *B) const;
};
/// Compare kind, literals and target.
friend bool operator==(const SPIRVDecorateGeneric &A,
const SPIRVDecorateGeneric &B);

SPIRVDecorationGroup *getOwner() const { return Owner; }

void setOwner(SPIRVDecorationGroup *Owner) { this->Owner = Owner; }
Expand Down Expand Up @@ -119,26 +110,7 @@ class SPIRVDecorateGeneric : public SPIRVAnnotationGeneric {
SPIRVDecorationGroup *Owner; // Owning decorate group
};

class SPIRVDecorateSet
: public std::multiset<SPIRVDecorateGeneric *,
SPIRVDecorateGeneric::Comparator> {
public:
typedef std::multiset<SPIRVDecorateGeneric *,
SPIRVDecorateGeneric::Comparator>
BaseType;
iterator insert(value_type &Dec) {
auto ER = BaseType::equal_range(Dec);
for (auto I = ER.first, E = ER.second; I != E; ++I) {
SPIRVDBG(spvdbgs() << "[compare decorate] " << *Dec << " vs " << **I
<< " : ");
if (**I == *Dec)
return I;
SPIRVDBG(spvdbgs() << " diff\n");
}
SPIRVDBG(spvdbgs() << "[add decorate] " << *Dec << '\n');
return BaseType::insert(Dec);
}
};
typedef std::vector<SPIRVDecorateGeneric *> SPIRVDecorateVec;

class SPIRVDecorate : public SPIRVDecorateGeneric {
public:
Expand Down Expand Up @@ -382,17 +354,17 @@ class SPIRVDecorationGroup : public SPIRVEntry {
void encodeAll(spv_ostream &O) const override;
_SPIRV_DCL_ENCDEC
// Move the given decorates to the decoration group
void takeDecorates(SPIRVDecorateSet &Decs) {
void takeDecorates(SPIRVDecorateVec &Decs) {
Decorations = std::move(Decs);
for (auto &I : Decorations)
const_cast<SPIRVDecorateGeneric *>(I)->setOwner(this);
Decs.clear();
}

SPIRVDecorateSet &getDecorations() { return Decorations; }
SPIRVDecorateVec &getDecorations() { return Decorations; }

protected:
SPIRVDecorateSet Decorations;
SPIRVDecorateVec Decorations;
void validate() const override {
assert(OpCode == OC);
assert(WordCount == WC);
Expand Down
24 changes: 24 additions & 0 deletions lib/SPIRV/libSPIRV/SPIRVError.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

#include "SPIRVDebug.h"
#include "SPIRVUtil.h"
#include "llvm/IR/Instruction.h"
#include <iostream>
#include <sstream>
#include <string>
Expand Down Expand Up @@ -109,12 +110,35 @@ class SPIRVErrorLog {
const std::string &DetailedMsg = "",
const char *CondString = nullptr,
const char *FileName = nullptr, unsigned LineNumber = 0);
// Check if Condition is satisfied and set ErrCode and DetailedMsg with Value
// text representation if not. Returns true if no error.
bool checkError(bool Condition, SPIRVErrorCode ErrCode, llvm::Value *Value,
const std::string &DetailedMsg = "",
const char *CondString = nullptr,
const char *FileName = nullptr, unsigned LineNumber = 0);

protected:
SPIRVErrorCode ErrorCode;
std::string ErrorMsg;
};

inline bool SPIRVErrorLog::checkError(bool Cond, SPIRVErrorCode ErrCode,
llvm::Value *Value,
const std::string &Msg,
const char *CondString,
const char *FileName, unsigned LineNo) {
// Do early exit to avoid expensive toString() function call unless it is
// actually needed. That speeds up translator's execution.
if (Cond)
return Cond;
// Do not overwrite previous failure.
if (ErrorCode != SPIRVEC_Success)
return Cond;
std::string ValueIR = toString(Value);
return checkError(Cond, ErrCode, Msg + "\n" + ValueIR, CondString, FileName,
LineNo);
}

inline bool SPIRVErrorLog::checkError(bool Cond, SPIRVErrorCode ErrCode,
const std::string &Msg,
const char *CondString,
Expand Down
16 changes: 8 additions & 8 deletions lib/SPIRV/libSPIRV/SPIRVModule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ class SPIRVModuleImpl : public SPIRVModule {
SPIRVStringVec StringVec;
SPIRVMemberNameVec MemberNameVec;
std::shared_ptr<const SPIRVLine> CurrentLine;
SPIRVDecorateSet DecorateSet;
SPIRVDecorateVec DecorateVec;
SPIRVDecGroupVec DecGroupVec;
SPIRVGroupDecVec GroupDecVec;
SPIRVAsmTargetVector AsmTargetVec;
Expand Down Expand Up @@ -1002,7 +1002,7 @@ SPIRVModuleImpl::addDecorate(SPIRVDecorateGeneric *Dec) {
(void)Found;
assert(Found && "Decorate target does not exist");
if (!Dec->getOwner())
DecorateSet.insert(Dec);
DecorateVec.push_back(Dec);
addCapabilities(Dec->getRequiredCapability());
return Dec;
}
Expand Down Expand Up @@ -1706,8 +1706,8 @@ spv_ostream &operator<<(spv_ostream &O, const std::vector<T *> &V) {
return O;
}

template <class T, class B>
spv_ostream &operator<<(spv_ostream &O, const std::multiset<T *, B> &V) {
template <class T, class B = std::less<T>>
spv_ostream &operator<<(spv_ostream &O, const std::unordered_set<T *, B> &V) {
for (auto &I : V)
O << *I;
return O;
Expand Down Expand Up @@ -1898,7 +1898,7 @@ spv_ostream &operator<<(spv_ostream &O, SPIRVModule &M) {
MI.ForwardPointerVec);

O << MI.MemberNameVec << MI.ModuleProcessedVec << MI.DecGroupVec
<< MI.DecorateSet << MI.GroupDecVec << MI.ForwardPointerVec << TS;
<< MI.DecorateVec << MI.GroupDecVec << MI.ForwardPointerVec << TS;

if (M.isAllowedToUseExtension(ExtensionID::SPV_INTEL_inline_assembly)) {
O << SPIRVNL() << MI.AsmTargetVec << MI.AsmVec;
Expand All @@ -1923,11 +1923,11 @@ SPIRVDecorationGroup *SPIRVModuleImpl::addDecorationGroup() {
SPIRVDecorationGroup *
SPIRVModuleImpl::addDecorationGroup(SPIRVDecorationGroup *Group) {
add(Group);
Group->takeDecorates(DecorateSet);
Group->takeDecorates(DecorateVec);
DecGroupVec.push_back(Group);
SPIRVDBG(spvdbgs() << "[addDecorationGroup] {" << *Group << "}\n";
spvdbgs() << " Remaining DecorateSet: {" << DecorateSet << "}\n");
assert(DecorateSet.empty());
spvdbgs() << " Remaining DecorateVec: {" << DecorateVec << "}\n");
assert(DecorateVec.empty());
return Group;
}

Expand Down
12 changes: 12 additions & 0 deletions lib/SPIRV/libSPIRV/SPIRVUtil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
#include <ostream>
#define spv_ostream std::ostream

#include "llvm/Support/raw_ostream.h"

#include <algorithm>
#include <cassert>
#include <cstdint>
Expand Down Expand Up @@ -422,6 +424,16 @@ getOrInsert(MapTy &Map, typename MapTy::key_type Key, FuncTy Func) {
return NF;
}

template <typename T> std::string toString(const T *Object) {
if (Object == nullptr)
return "";
std::string S;
llvm::raw_string_ostream RSOS(S);
Object->print(RSOS);
RSOS.flush();
return S;
}

} // namespace SPIRV

#endif // SPIRV_LIBSPIRV_SPIRVUTIL_H
Loading

0 comments on commit 2b19c10

Please sign in to comment.