Skip to content

Commit 3ddceed

Browse files
fda0igcbot
fda0
authored and
igcbot
committed
Various IGC refactors
Pass arguments as references. Iterate in loops using references. Fix potential nullptr dereferences. Conform to rule of three or rule of five. Add `[[fallthrough]]` attributes to switch statements. - In `IGC/AdaptorOCL/ocl_igc_interface/impl/igc_ocl_device_ctx_impl.h` fix a race condition.
1 parent f505af2 commit 3ddceed

File tree

17 files changed

+69
-19
lines changed

17 files changed

+69
-19
lines changed

‎3d/common/iStdLib/File.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ inline int CreateAppOutputDir(
598598
{
599599
pathBuf[0] = '\0';
600600
free(path);
601+
free(outputDirectoryCopy);
601602
return ret;
602603
}
603604
}
@@ -607,6 +608,7 @@ inline int CreateAppOutputDir(
607608
{
608609
pathBuf[0]= '\0';
609610
free(path);
611+
free(outputDirectoryCopy);
610612
return ret;
611613
}
612614
}
@@ -630,6 +632,7 @@ inline int CreateAppOutputDir(
630632
}
631633

632634
free(path);
635+
free(outputDirectoryCopy);
633636
#endif //#elif !defined(_WIN32)
634637
return ret;
635638
}

‎IGC/AdaptorCommon/RayTracing/RayTracingAddressSpaceAliasAnalysis.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ class RayTracingAddressSpaceAAResult : public llvm::AAResultBase<RayTracingAddre
3434
RayTracingAddressSpaceAAResult(RayTracingAddressSpaceAAResult&& Arg)
3535
: llvm::AAResultBase<RayTracingAddressSpaceAAResult>(std::move(Arg)),
3636
TLI(Arg.TLI), CGC(Arg.CGC), allStateful(checkStateful(Arg.CGC)) {}
37+
RayTracingAddressSpaceAAResult(const RayTracingAddressSpaceAAResult&) = delete;
38+
RayTracingAddressSpaceAAResult& operator=(const RayTracingAddressSpaceAAResult&) = delete;
39+
RayTracingAddressSpaceAAResult& operator=(RayTracingAddressSpaceAAResult&&) = delete;
3740

3841
IGCLLVM::AliasResultEnum alias(
3942
const llvm::MemoryLocation& LocA, const llvm::MemoryLocation& LocB

‎IGC/AdaptorOCL/CLElfLib/ElfWriter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ CElfWriter::CElfWriter(
2626
m_dataSize = 0;
2727
m_numSections = 0;
2828
m_stringTableSize = 0;
29+
m_totalBinarySize = 0;
2930
}
3031

3132
/******************************************************************************\

‎IGC/AdaptorOCL/OCL/sp/spp_g8.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ class CGen8OpenCLProgramBase : DisallowCopy {
4949
const CGen8OpenCLStateProcessor::IProgramContext& PI,
5050
const WA_TABLE& WATable);
5151
virtual ~CGen8OpenCLProgramBase();
52+
CGen8OpenCLProgramBase(const CGen8OpenCLProgramBase&) = delete;
53+
CGen8OpenCLProgramBase& operator=(const CGen8OpenCLProgramBase&) = delete;
5254

5355
/// GetProgramBinary - getting legacy (Patch token based) binary format
5456
/// Write program header and the already written patch token info
@@ -100,6 +102,8 @@ class CGen8OpenCLProgram : public CGen8OpenCLProgramBase
100102
CGen8OpenCLProgram(PLATFORM platform, const IGC::OpenCLProgramContext &context);
101103

102104
~CGen8OpenCLProgram();
105+
CGen8OpenCLProgram(const CGen8OpenCLProgram&) = delete;
106+
CGen8OpenCLProgram& operator=(const CGen8OpenCLProgram&) = delete;
103107

104108
/// API for getting legacy (Patch token based) binary
105109
/// CreateKernelBinaries - write patch token information and gen binary to

‎IGC/AdaptorOCL/OCL/sp/zebin_builder.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -421,17 +421,17 @@ void ZEBinaryBuilder::addProgramSymbols(const IGC::SOpenCLProgramInfo& annotatio
421421

422422
// add symbols defined in global constant section
423423
IGC_ASSERT(symbols.globalConst.empty() || mGlobalConstSectID != -1);
424-
for (auto sym : symbols.globalConst)
424+
for (const auto& sym : symbols.globalConst)
425425
addSymbol(sym, llvm::ELF::STB_GLOBAL, mGlobalConstSectID);
426426

427427
// add symbols defined in global string constant section
428428
IGC_ASSERT(symbols.globalStringConst.empty() || mConstStringSectID != -1);
429-
for (auto sym : symbols.globalStringConst)
429+
for (const auto& sym : symbols.globalStringConst)
430430
addSymbol(sym, llvm::ELF::STB_GLOBAL, mConstStringSectID);
431431

432432
// add symbols defined in global section, mGlobalSectID may be unallocated
433433
// at this point if symbols are undef
434-
for (auto sym : symbols.global)
434+
for (const auto& sym : symbols.global)
435435
addSymbol(sym, llvm::ELF::STB_GLOBAL, mGlobalSectID);
436436
}
437437

@@ -453,7 +453,7 @@ void ZEBinaryBuilder::addKernelSymbols(
453453
annotations.m_kernelProgram);
454454

455455
// add local symbols of this kernel binary
456-
for (auto sym : symbols.local) {
456+
for (const auto& sym : symbols.local) {
457457
IGC_ASSERT(sym.s_type != vISA::GenSymType::S_UNDEF);
458458
addSymbol(sym, llvm::ELF::STB_LOCAL, kernelSectId);
459459
}
@@ -477,7 +477,7 @@ void ZEBinaryBuilder::addProgramRelocations(const IGC::SOpenCLProgramInfo& annot
477477
mBuilder.addRelRelocation(reloc.r_offset, reloc.r_symbol, static_cast<zebin::R_TYPE_ZEBIN>(reloc.r_type), mGlobalConstSectID);
478478

479479
IGC_ASSERT(relocs.globalReloc.empty() || mGlobalSectID != -1);
480-
for (auto reloc : relocs.globalReloc)
480+
for (const auto& reloc : relocs.globalReloc)
481481
mBuilder.addRelRelocation(reloc.r_offset, reloc.r_symbol, static_cast<zebin::R_TYPE_ZEBIN>(reloc.r_type), mGlobalSectID);
482482
}
483483

@@ -500,7 +500,7 @@ void ZEBinaryBuilder::addKernelRelocations(
500500
// FIXME: For r_type, zebin::R_TYPE_ZEBIN should have the same enum value as visa::GenRelocType.
501501
// Take the value directly
502502
if (!relocs.empty())
503-
for (auto reloc : relocs)
503+
for (const auto& reloc : relocs)
504504
mBuilder.addRelRelocation(reloc.r_offset, reloc.r_symbol, (zebin::R_TYPE_ZEBIN)reloc.r_type, targetId);
505505
}
506506

@@ -838,7 +838,7 @@ void ZEBinaryBuilder::addElfSections(void* elfBin, size_t elfSize)
838838

839839
// Avoid symbol duplications - check whether a current symbol has been previously added.
840840
bool isSymbolAdded = false;
841-
for (auto zeBinSym : zeBinSymbols)
841+
for (const auto& zeBinSym : zeBinSymbols)
842842
{
843843
if (!zeBinSym.compare(zeSym.s_name))
844844
{

‎IGC/AdaptorOCL/cif/cif/macros/enable.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ SPDX-License-Identifier: MIT
3939
template<typename ... ArgsT> \
4040
NAME(ArgsT &&... args); \
4141
NAME(Impl * pImpl); \
42+
NAME(const NAME&) = delete; \
43+
NAME& operator=(const NAME&) = delete; \
44+
NAME& operator=(NAME&&) = delete; \
4245
}; \
4346
using NAME##Base = NAME<CIF::BaseVersion>;
4447

@@ -62,6 +65,9 @@ SPDX-License-Identifier: MIT
6265
template<typename ... ArgsT> \
6366
NAME(ArgsT &&... args); \
6467
NAME(Impl * pImpl); \
68+
NAME(const NAME&) = delete; \
69+
NAME& operator=(const NAME&) = delete; \
70+
NAME& operator=(NAME&&) = delete; \
6571
}; \
6672
using NAME##Base = NAME<CIF::BaseVersion>;
6773

‎IGC/AdaptorOCL/ocl_igc_interface/impl/igc_ocl_device_ctx_impl.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,23 +127,30 @@ CIF_DECLARE_INTERFACE_PIMPL(IgcOclDeviceCtx) : CIF::PimplBase
127127

128128
const IGC::CPlatform & GetIgcCPlatform()
129129
{
130+
// "fast path" where we return initialized igcPlatform without a lock
130131
if(igcPlatform.get() != nullptr)
131132
{
132133
return *igcPlatform;
133134
}
134135

136+
// acquire a lock to if igcPlatform is uninitialized
135137
std::lock_guard<std::mutex> lock{this->mutex};
136138

139+
// check if other thread initialized igcPlatform first
137140
if(igcPlatform.get() != nullptr)
138141
{
139142
return *igcPlatform;
140143
}
141144

142-
igcPlatform.reset(new IGC::CPlatform(platform->p));
143-
igcPlatform->SetGTSystemInfo(gtSystemInfo->gsi);
144-
igcPlatform->setOclCaps(igcFeaturesAndWorkarounds->OCLCaps);
145-
IGC::SetWorkaroundTable(&igcFeaturesAndWorkarounds->FeTable, igcPlatform.get());
146-
IGC::SetCompilerCaps(&igcFeaturesAndWorkarounds->FeTable, igcPlatform.get());
145+
// construct fully initialized CPlatform object
146+
IGC::CPlatform *new_platform = new IGC::CPlatform(platform->p);
147+
new_platform->SetGTSystemInfo(gtSystemInfo->gsi);
148+
new_platform->setOclCaps(igcFeaturesAndWorkarounds->OCLCaps);
149+
IGC::SetWorkaroundTable(&igcFeaturesAndWorkarounds->FeTable, new_platform);
150+
IGC::SetCompilerCaps(&igcFeaturesAndWorkarounds->FeTable, new_platform);
151+
// assign new_platform to igcPlatform after a full initialization
152+
// is finished to avoid race conditions
153+
igcPlatform.reset(new_platform);
147154
return *igcPlatform;
148155
}
149156

‎IGC/AdaptorOCL/ocl_igc_interface/impl/igc_ocl_translation_ctx_impl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ struct IGC_State {
124124
~IGC_State() {
125125
isAlive = false;
126126
}
127+
IGC_State(const IGC_State&) = delete;
128+
IGC_State& operator=(const IGC_State&) = delete;
127129
static const bool isDestructed() {
128130
static const IGC_State obj;
129131
return !obj.isAlive;

‎IGC/Compiler/CISACodeGen/BlockMemOpAddrScalarizationPass.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ bool BlockMemOpAddrScalarizationPass::scalarizeAddrArithmForBlockRdWr(GenIntrins
290290
}
291291
}
292292
// Update instructions list for next check.
293-
InstrVector = NewInstrVector;
293+
InstrVector = std::move(NewInstrVector);
294294
}
295295

296296
// Insert broadcast instructions

‎IGC/Compiler/CISACodeGen/CISABuilder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7673,6 +7673,7 @@ namespace IGC
76737673
{
76747674
case GenISAIntrinsic::GenISA_vaErode:
76757675
erodeDilateMode = VA_ERODE;
7676+
[[fallthrough]];
76767677
case GenISAIntrinsic::GenISA_vaDilate:
76777678
V(vKernel->AppendVISAVAErodeDilate(erodeDilateMode, samplerHnd, surfaceOpnd, uOffset, vOffset, execMode, vaOutput));
76787679
break;

‎IGC/Compiler/CISACodeGen/CVariable.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ namespace IGC {
4343
CName() : CName(nullptr) { }
4444

4545
CName(const CName &) = default;
46+
CName& operator=(const CName&) = default;
4647

4748
CName(const std::string &arg)
4849
#ifdef IGC_MAP_LLVM_NAMES_TO_VISA

‎IGC/Compiler/CISACodeGen/IGCLivenessAnalysis.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ void IGCLivenessAnalysis::printPhi(const PhiSet &Set, std::string &Output) {
114114
return;
115115

116116
Output += "PHIS:\t[\t" + std::to_string(Set.size()) + "\t] ";
117-
for (auto Elem : Set) {
117+
for (const auto& Elem : Set) {
118118
auto &OneBBSet = Elem.second;
119119
unsigned int Size = OneBBSet.size();
120120
if (PrinterType > 1) {

‎IGC/Compiler/CISACodeGen/RegisterPressureEstimate.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ namespace IGC
8686
LiveRange() {}
8787

8888
LiveRange(const LiveRange&) = delete;
89+
LiveRange& operator=(const LiveRange&) = delete;
8990

9091
/// \brief Shorten a segment.
9192
void setBegin(unsigned B);

‎IGC/Compiler/CISACodeGen/SLMConstProp.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ namespace IGC
104104
clear();
105105
}
106106

107+
SymbolicEvaluation(const SymbolicEvaluation&) = delete;
108+
SymbolicEvaluation& operator=(const SymbolicEvaluation&) = delete;
109+
107110
// Return a Canonicalized Polynomial Expression.
108111
SymExpr* getSymExpr(const llvm::Value* V);
109112

‎IGC/Compiler/Optimizer/OpenCLPasses/AddressSpaceAliasAnalysis/AddressSpaceAliasAnalysis.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ namespace {
3030
AddressSpaceAAResult(AddressSpaceAAResult&& Arg)
3131
: AAResultBase(std::move(Arg)), TLI(Arg.TLI), CGC(Arg.CGC) {}
3232

33+
AddressSpaceAAResult(const AddressSpaceAAResult&) = delete;
34+
AddressSpaceAAResult& operator=(const AddressSpaceAAResult&) = delete;
35+
AddressSpaceAAResult& operator=(AddressSpaceAAResult&&) = delete;
36+
3337
IGCLLVM::AliasResultEnum alias(const MemoryLocation& LocA, const MemoryLocation& LocB
3438
#if LLVM_VERSION_MAJOR >= 9
3539
, AAQueryInfo & AAQI

‎IGC/Metrics/IGCMetric.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ namespace IGCMetrics
3737
public:
3838
IGCMetric();
3939
~IGCMetric();
40+
IGCMetric(const IGCMetric&) = delete;
41+
IGCMetric& operator=(const IGCMetric&) = delete;
4042
bool Enable();
4143
void Init(ShaderHash* Hash, bool isDebugInfo);
4244

‎IGC/common/SystemThread.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,10 @@ struct Xe2DebugSurfaceLayout
138138
static constexpr size_t DBG_REG_ELEMENT_SIZE = 4;
139139
static constexpr size_t DBG_REG_ALIGN = 24;
140140

141-
142-
// Number of Registers: 3
143-
// Elements: 16 or 2
144-
// Element Size: 32 bits
141+
142+
// Number of Registers: 3
143+
// Elements: 16 or 2
144+
// Element Size: 32 bits
145145
// fc0.0-fc0.15
146146
// fc1.0-fc1.15
147147
// fc2.0 fc2.1, All other encodings like fc2.2 to f2.15 are reserved.
@@ -172,7 +172,7 @@ struct Xe2DebugSurfaceLayout
172172
uint8_t fc[FC_COUNT * FC_ELEMENTS * FC_ELEMENT_SIZE + FC_ALIGN];
173173
};
174174

175-
struct StateSaveAreaHeader Xe2SIPCSRDebugBindlessDebugHeader =
175+
struct StateSaveAreaHeader Xe2SIPCSRDebugBindlessDebugHeader =
176176
{
177177
{"tssarea", 0, {2, 0, 0}, sizeof(StateSaveAreaHeader) / 8, {0, 0, 0}}, // versionHeader
178178
{
@@ -928,6 +928,18 @@ bool CSystemThread::CreateSystemThreadKernel(
928928
delete pKernelProgram;
929929
}
930930
}
931+
if (!success)
932+
{
933+
if (pSystemThreadKernelOutput)
934+
{
935+
IGC::aligned_free(pSystemThreadKernelOutput->m_pKernelProgram);
936+
pSystemThreadKernelOutput->m_pKernelProgram = nullptr;
937+
IGC::aligned_free(pSystemThreadKernelOutput->m_pStateSaveAreaHeader);
938+
pSystemThreadKernelOutput->m_pStateSaveAreaHeader = nullptr;
939+
delete pSystemThreadKernelOutput;
940+
pSystemThreadKernelOutput = nullptr;
941+
}
942+
}
931943
return success;
932944
}
933945

0 commit comments

Comments
 (0)