Skip to content

Commit 53187e9

Browse files
dlei6gsys_zuul
dlei6g
authored and
sys_zuul
committed
Revert of commit: 262d8aeceb2c15b0b7d58d48b17a5fed9c2450b5
Fixed analysis for function pointer and global variable symbols - Fixed FGA for function pointer analysis, such that indirect calls are treated like stack calls. - Changed getUniqueEntryFunc to handle multiple entry functions, and the same entry is returned each time. - Re-introduced symbol table entry for global variables removed in an earlier commit. Change-Id: If77e68e9d0a55d21c32bb568011f5f0141c67a53
1 parent 6e2681f commit 53187e9

File tree

10 files changed

+65
-89
lines changed

10 files changed

+65
-89
lines changed

‎IGC/AdaptorCommon/ProcessFuncAttributes.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,12 @@ bool ProcessFuncAttributes::runOnModule(Module& M)
221221
F->addFnAttr("IndirectlyCalled");
222222
F->addFnAttr("visaStackCall");
223223
// Require global relocation if any global values are used in indirect functions, since we cannot pass implicit args
224-
if (!F->isDeclaration())
224+
F->addFnAttr("EnableGlobalRelocation");
225+
226+
if (IGC_GET_FLAG_VALUE(FunctionControl) == FLAG_FCALL_FORCE_INDIRECTCALL)
225227
{
226-
F->addFnAttr("EnableGlobalRelocation");
227-
if (IGC_GET_FLAG_VALUE(FunctionControl) == FLAG_FCALL_FORCE_INDIRECTCALL)
228-
{
229-
F->removeFnAttr(llvm::Attribute::AlwaysInline);
230-
F->addFnAttr(llvm::Attribute::NoInline);
231-
}
228+
F->removeFnAttr(llvm::Attribute::AlwaysInline);
229+
F->addFnAttr(llvm::Attribute::NoInline);
232230
}
233231
};
234232

‎IGC/Compiler/CISACodeGen/CISABuilder.cpp

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4638,26 +4638,18 @@ namespace IGC
46384638
for (auto global : modMD->inlineProgramScopeOffsets)
46394639
{
46404640
GlobalVariable* pGlobal = global.first;
4641-
4642-
// Create symbol for external globals
4643-
bool needSymbol = (pGlobal->getLinkage() == GlobalValue::ExternalLinkage) || (pGlobal->getLinkage() == GlobalValue::CommonLinkage);
4644-
if (!needSymbol)
4641+
bool needRelocation = false;
4642+
for (auto ui = pGlobal->user_begin(), ue = pGlobal->user_end(); ui != ue; ui++)
46454643
{
4646-
pGlobal->removeDeadConstantUsers();
4647-
// Also need to create symbol if it requires relocation
4648-
for (auto ui = pGlobal->user_begin(), ue = pGlobal->user_end(); ui != ue; ui++)
4644+
// Check if need relocation
4645+
Instruction* inst = dyn_cast<Instruction>(*ui);
4646+
if (inst && inst->getParent()->getParent()->hasFnAttribute("EnableGlobalRelocation"))
46494647
{
4650-
// Check if need relocation
4651-
Instruction* inst = dyn_cast<Instruction>(*ui);
4652-
if (inst && inst->getParent()->getParent()->hasFnAttribute("EnableGlobalRelocation"))
4653-
{
4654-
needSymbol = true;
4655-
break;
4656-
}
4648+
needRelocation = true;
4649+
break;
46574650
}
46584651
}
4659-
4660-
if (needSymbol)
4652+
if (needRelocation)
46614653
{
46624654
StringRef name = pGlobal->getName();
46634655
unsigned addrSpace = pGlobal->getType()->getAddressSpace();

‎IGC/Compiler/CISACodeGen/EmitVISAPass.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,8 @@ bool EmitPass::canCompileCurrentShader(llvm::Function& F)
247247
{
248248
CodeGenContext* ctx = getAnalysis<CodeGenContextWrapper>().getCodeGenContext();
249249

250-
// If uses subroutines/stackcall, we can only compile a single SIMD mode
251-
if (m_FGA && (!m_FGA->getGroup(&F)->isSingle() || m_FGA->getGroup(&F)->hasStackCall()))
250+
// If uses subroutines, we can only compile a single SIMD mode
251+
if (m_FGA && (!m_FGA->getGroup(&F)->isSingle() || m_FGA->getGroup(&F)->hasExternFCall()))
252252
{
253253
// default SIMD8
254254
SIMDMode compiledSIMD = SIMDMode::SIMD8;
@@ -359,7 +359,7 @@ void EmitPass::CreateKernelShaderMap(CodeGenContext* ctx, MetaDataUtils* pMdUtil
359359
{
360360
// Single PS
361361
// Assuming single shader information in metadata
362-
Function* pFunc = getUniqueEntryFunc(pMdUtils, ctx->getModuleMetaData());
362+
Function* pFunc = getUniqueEntryFunc(pMdUtils);
363363

364364
CShaderProgram* pProgram = new CShaderProgram(ctx, pFunc);
365365
m_shaders[pFunc] = pProgram;
@@ -457,6 +457,7 @@ bool EmitPass::runOnFunction(llvm::Function& F)
457457
}
458458

459459
bool hasStackCall = m_FGA && m_FGA->getGroup(&F)->hasStackCall();
460+
bool hasFunctionPointer = m_pCtx->m_instrTypes.hasIndirectCall || (m_FGA && m_FGA->getGroup(&F)->hasExternFCall());
460461
bool ptr64bits = (m_DL->getPointerSizeInBits(ADDRESS_SPACE_PRIVATE) == 64);
461462
if (!m_FGA || m_FGA->isGroupHead(&F))
462463
{
@@ -471,7 +472,7 @@ bool EmitPass::runOnFunction(llvm::Function& F)
471472
m_encoder->InitEncoder(m_canAbortOnSpill, hasStackCall);
472473
initDefaultRoundingMode();
473474
m_currShader->PreCompile();
474-
if (hasStackCall)
475+
if (hasStackCall || hasFunctionPointer)
475476
{
476477
CVariable* pStackBase = nullptr;
477478
CVariable* pStackSize = nullptr;
@@ -746,21 +747,19 @@ bool EmitPass::runOnFunction(llvm::Function& F)
746747
IF_DEBUG_INFO_IF(m_currShader->diData, m_currShader->diData->markOutput(F, m_currShader);)
747748
IF_DEBUG_INFO_IF(m_currShader->diData, m_currShader->diData->addVISAModule(&F, m_pDebugEmitter->GetVISAModule());)
748749

749-
// Compile only when this is the last function for this kernel.
750-
bool finalize = (!m_FGA || m_FGA->isGroupTail(&F));
750+
// Compile only when this is the last function for this kernel.
751+
bool finalize = (!m_FGA || m_FGA->isGroupTail(&F));
751752
bool destroyVISABuilder = false;
752753
if (finalize)
753754
{
754755
destroyVISABuilder = true;
755-
// We only need one symbol table per module. If there are multiple entry functions, only create a symbol
756-
// table for the unique entry function
757-
Function* uniqueEntry = getUniqueEntryFunc(pMdUtils, m_moduleMD);
758-
Function* currHead = m_FGA ? m_FGA->getGroupHead(&F) : &F;
759-
bool compileWithSymbolTable = (currHead == uniqueEntry);
756+
// We only need one symbol table per module. If there are multiple kernels, only create a symbol
757+
// table for the one with indirectly called functions attached.
758+
bool compileWithSymbolTable = !m_FGA || (m_FGA->getGroup(&F)->hasIndirectFuncs());
760759
m_encoder->Compile(compileWithSymbolTable);
761760
// if we are doing stack-call, do the following:
762761
// - Hard-code a large scratch-space for visa
763-
if (hasStackCall)
762+
if (hasStackCall || hasFunctionPointer)
764763
{
765764
if (m_currShader->ProgramOutput()->m_scratchSpaceUsedBySpills == 0)
766765
{

‎IGC/Compiler/CISACodeGen/GenCodeGenModule.cpp

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -412,9 +412,6 @@ bool GenXCodeGenModule::runOnModule(Module& M)
412412
delete SCCNodes;
413413
}
414414

415-
// Before adding indirect functions to groups, check and set stack call for each group
416-
FGA->setGroupStackCall();
417-
418415
// Add all indirect functions to the default kernel group
419416
FGA->addIndirectFuncsToKernelGroup(&M);
420417

@@ -534,43 +531,45 @@ bool GenXFunctionGroupAnalysis::useStackCall(llvm::Function* F)
534531
void GenXFunctionGroupAnalysis::addIndirectFuncsToKernelGroup(llvm::Module* pModule)
535532
{
536533
auto pMdUtils = getAnalysis<MetaDataUtilsWrapper>().getMetaDataUtils();
537-
auto modMD = getAnalysis<MetaDataUtilsWrapper>().getModuleMetaData();
538534

539-
Function* defaultKernel = getUniqueEntryFunc(pMdUtils, modMD);
535+
Function* defaultKernel = nullptr;
536+
// Set the first kernel as the default
537+
for (auto I = pModule->begin(), E = pModule->end(); I != E; ++I)
538+
{
539+
Function* F = &(*I);
540+
if (isEntryFunc(pMdUtils, F))
541+
{
542+
defaultKernel = F;
543+
break;
544+
}
545+
}
540546
assert(defaultKernel && "kernel does not exist in this group");
541547

542548
FunctionGroup* defaultFG = getGroupForHead(defaultKernel);
543549
assert(defaultFG && "default kernel group does not exist");
544550

551+
SmallVector<Function*, 8> indirectFuncs;
545552
// Add all externally linked functions into the default kernel group
546553
for (auto I = pModule->begin(), E = pModule->end(); I != E; ++I)
547554
{
548555
Function* F = &(*I);
549-
if (F->hasFnAttribute("IndirectlyCalled") && !isEntryFunc(pMdUtils, F))
556+
if (F->hasFnAttribute("IndirectlyCalled"))
550557
{
551558
if (!F->isDeclaration())
552559
addToFunctionGroup(F, defaultFG, F);
560+
defaultFG->m_hasIndirectFuncs = true;
561+
indirectFuncs.push_back(F);
553562
}
554563
}
555-
// If functions are indirectly called, treat it as stack call
556-
for (auto I = pModule->begin(), E = pModule->end(); I != E; ++I)
564+
for (auto F : indirectFuncs)
557565
{
558-
Function* F = &(*I);
559-
auto FG = getGroup(F);
560-
if (FG && FG->m_hasStackCall == false)
566+
// Mark caller group if it uses an indirect function
567+
for (auto U : F->users())
561568
{
562-
for (auto ii = inst_begin(F), ei = inst_end(F); ii != ei; ii++)
569+
if (Instruction * I = dyn_cast<Instruction>(U))
563570
{
564-
if (CallInst* call = dyn_cast<CallInst>(&*ii))
565-
{
566-
if (call->isInlineAsm()) continue;
567-
Function* calledF = call->getCalledFunction();
568-
if (!calledF || calledF->hasFnAttribute("IndirectlyCalled"))
569-
{
570-
FG->m_hasStackCall = true;
571-
break;
572-
}
573-
}
571+
Function* Caller = I->getParent()->getParent();
572+
getGroup(Caller)->m_hasExternFCall = true;
574573
}
575574
}
576575
}
@@ -619,9 +618,6 @@ bool GenXFunctionGroupAnalysis::rebuild(llvm::Module* Mod) {
619618
}
620619
}
621620

622-
// Reset stack call flag
623-
setGroupStackCall();
624-
625621
// Re-add all indirect functions to the default kernel group
626622
addIndirectFuncsToKernelGroup(Mod);
627623

‎IGC/Compiler/CISACodeGen/GenCodeGenModule.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,14 @@ namespace IGC {
9494
bool isSingle() {
9595
return (Functions.size() == 1 && Functions.front()->size() == 1);
9696
}
97-
bool hasStackCall() { return m_hasStackCall; }
97+
bool hasStackCall() {
98+
return (Functions.size() > 1);
99+
}
100+
/// \brief Indicate if any function in this group directly calls an externally linked function
101+
bool hasExternFCall() { return m_hasExternFCall; }
102+
103+
/// \brief Indicate if there are indirectly functions attached to this group
104+
bool hasIndirectFuncs() { return m_hasIndirectFuncs; }
98105

99106
void replaceGroupHead(llvm::Function* OH, llvm::Function* NH) {
100107
auto headSG = Functions[0];
@@ -104,7 +111,8 @@ namespace IGC {
104111
}
105112

106113
private:
107-
bool m_hasStackCall = false;
114+
bool m_hasExternFCall = false;
115+
bool m_hasIndirectFuncs = false;
108116
};
109117

110118
class GenXFunctionGroupAnalysis : public llvm::ImmutablePass {
@@ -175,12 +183,6 @@ namespace IGC {
175183
SubGroupMap[F] = SubGroupHead;
176184
}
177185

178-
void setGroupStackCall() {
179-
for (auto FG : Groups) {
180-
FG->m_hasStackCall = (FG->Functions.size() > 1);
181-
}
182-
}
183-
184186
/// \brief Check whether this is a group header.
185187
bool isGroupHead(llvm::Function* F) {
186188
return getGroupForHead(F) != nullptr;

‎IGC/Compiler/CISACodeGen/PixelShaderCodeGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,7 @@ namespace IGC
13341334
// Single PS
13351335
CodeGen(ctx, shaders);
13361336
// Assuming single shader information in metadata
1337-
Function* pFunc = getUniqueEntryFunc(pMdUtils, ctx->getModuleMetaData());
1337+
Function* pFunc = getUniqueEntryFunc(pMdUtils);
13381338
// gather data to send back to the driver
13391339
shaders[pFunc]->FillProgram(&ctx->programOutput);
13401340
COMPILER_SHADER_STATS_PRINT(shaders[pFunc]->m_shaderStats, ShaderType::PIXEL_SHADER, ctx->hash, "");

‎IGC/Compiler/CISACodeGen/helper.cpp

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,10 +1779,9 @@ namespace IGC
17791779
IGC_IS_FLAG_DISABLED(DisableDSDualPatch);
17801780
}
17811781

1782-
Function* getUniqueEntryFunc(const IGCMD::MetaDataUtils* pM, IGC::ModuleMetaData* pModMD)
1782+
Function* getUniqueEntryFunc(const IGCMD::MetaDataUtils* pM)
17831783
{
17841784
Function* entryFunc = nullptr;
1785-
auto& FuncMD = pModMD->FuncMD;
17861785
for (auto i = pM->begin_FunctionsInfo(), e = pM->end_FunctionsInfo(); i != e; ++i)
17871786
{
17881787
IGCMD::FunctionInfoMetaDataHandle Info = i->second;
@@ -1791,23 +1790,17 @@ namespace IGC
17911790
continue;
17921791
}
17931792

1794-
Function* F = i->first;
1793+
const Function* F = i->first;
17951794
if (!entryFunc)
17961795
{
1797-
entryFunc = F;
1796+
entryFunc = const_cast<Function*>(F);
17981797
}
1799-
1800-
auto fi = FuncMD.find(F);
1801-
assert(fi != FuncMD.end());
1802-
if (fi->second.isUniqueEntry)
1798+
else
18031799
{
1804-
return F;
1800+
assert(false && "Not a single entry func!");
18051801
}
18061802
}
18071803
assert(entryFunc && "No entry func!");
1808-
auto ei = FuncMD.find(entryFunc);
1809-
assert(ei != FuncMD.end());
1810-
ei->second.isUniqueEntry = true;
18111804
return entryFunc;
18121805
}
18131806

‎IGC/Compiler/CISACodeGen/helper.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,8 @@ namespace IGC
218218
}
219219

220220
// Return a unique entry function.
221-
// If more than one entry exists, return the first and and set it as unique.
222-
// All subsequent calls to this function will get the entry set by the first call.
223-
llvm::Function* getUniqueEntryFunc(const IGCMD::MetaDataUtils* pM, IGC::ModuleMetaData* pModMD);
221+
// Assert if more than one entry function exists.
222+
llvm::Function* getUniqueEntryFunc(const IGCMD::MetaDataUtils* pM);
224223

225224
// \brief Get next instruction, returning null if it's the last of the BB.
226225
// This is the replacement of Instruction::getNextNode(), since getNextNode()

‎IGC/Compiler/Optimizer/OpenCLPasses/ProgramScopeConstants/ProgramScopeConstantAnalysis.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,8 @@ bool ProgramScopeConstantAnalysis::runOnModule(Module& M)
110110
continue;
111111
}
112112

113-
// If this variable isn't used and not exposed externally, don't add it to the buffer.
114-
if (globalVar->use_empty() &&
115-
globalVar->getLinkage() != GlobalValue::CommonLinkage &&
116-
globalVar->getLinkage() != GlobalValue::ExternalLinkage)
113+
// If this variable isn't used, don't add it to the buffer.
114+
if (globalVar->use_empty())
117115
{
118116
continue;
119117
}

‎IGC/common/MDFrameWork.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,6 @@ namespace IGC
158158
bool groupIDPresent = false;
159159
int privateMemoryPerWI = 0;
160160
bool globalIDPresent = false;
161-
bool isUniqueEntry = false;
162161

163162
std::vector<std::string> UserAnnotations;
164163

0 commit comments

Comments
 (0)