Skip to content

Commit adcbc20

Browse files
aus-intelsys_zuul
authored andcommitted
Fix incorrect code movement in CMABI
replaceUsesWithinFunction incorrectly moved code trying to fix phi issues that never arised. Remove fixing code as it really looks like it should not be here at all: 1) breakConstantExprs is called before and it removes all constantExprs from module; 2) only uses that are instructions are analyzed so it is impossible to change global inside constexpr to alloca. The only possible way to break things is to add another SCC pass immediately before CMABI that will restore constexpr as CMABI breaks them in doInitialization. Though, this should be fixed with redesign of CMABI and doing all the work in "run" function. Change-Id: I24ac5e3c8977977ac782e9a6c01ee238948b3d5d
1 parent 1c76396 commit adcbc20

File tree

1 file changed

+2
-29
lines changed
  • IGC/VectorCompiler/lib/GenXOpts/CMTrans

1 file changed

+2
-29
lines changed

IGC/VectorCompiler/lib/GenXOpts/CMTrans/CMABI.cpp

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -482,42 +482,19 @@ bool CMABI::runOnSCC(CallGraphSCC &SCC) {
482482
return Changed;
483483
}
484484

485-
// Sometimes we can get phi with GEP (or maybe some other inst) as an argument.
486-
// While GEP's arguments are constants, its OK as GEP is a constant to.
487-
// But when we replace constants with lokals, GEP becomes a normal instruction,
488-
// a normal instruction, that is placed before phi - wrong IR, we need to fix
489-
// it. Here it is fixed.
490-
static void fixPhiUseIssue(Instruction *Inst) {
491-
auto PhiUse = cast<PHINode>(Inst->use_begin()->getUser());
492-
auto InstOpNoInPhi = Inst->use_begin()->getOperandNo();
493-
IGC_ASSERT(Inst->getParent() == PhiUse->getParent());
494-
Inst->removeFromParent();
495-
Inst->insertBefore(PhiUse->getIncomingBlock(InstOpNoInPhi)->getTerminator());
496-
}
497-
498485
// Replace uses of global variables with the corresponding allocas with a
499486
// specified function.
500-
//
501-
// Returns vector of instructions with phi use, that should be later fixed.
502-
static std::vector<Instruction *>
487+
static void
503488
replaceUsesWithinFunction(SmallDenseMap<Value *, Value *> &GlobalsToReplace,
504489
Function *F) {
505-
std::vector<Instruction *> PhiUseIssueInsts;
506490
for (auto I = inst_begin(F), E = inst_end(F); I != E; ++I) {
507491
Instruction *Inst = &*I;
508492
for (unsigned i = 0, e = Inst->getNumOperands(); i < e; ++i) {
509493
auto Iter = GlobalsToReplace.find(Inst->getOperand(i));
510494
if (Iter != GlobalsToReplace.end())
511495
Inst->setOperand(i, Iter->second);
512496
}
513-
if (Inst->getNumUses() == 1) {
514-
auto PhiUse = dyn_cast<PHINode>(Inst->use_begin()->getUser());
515-
if (PhiUse && Inst->getParent() == PhiUse->getParent()) {
516-
PhiUseIssueInsts.push_back(Inst);
517-
}
518-
}
519497
}
520-
return PhiUseIssueInsts;
521498
}
522499

523500
// \brief Create allocas for globals directly used in this kernel and
@@ -544,11 +521,7 @@ void CMABI::LocalizeGlobals(LocalizationInfo &LI) {
544521
}
545522

546523
// Replaces all globals uses within this function.
547-
auto PhiUseIssueInsts = replaceUsesWithinFunction(GlobalsToReplace, Fn);
548-
549-
for (auto InstWithPhiUse : PhiUseIssueInsts) {
550-
fixPhiUseIssue(InstWithPhiUse);
551-
}
524+
replaceUsesWithinFunction(GlobalsToReplace, Fn);
552525
}
553526

554527
CallGraphNode *CMABI::ProcessNode(CallGraphNode *CGN) {

0 commit comments

Comments
 (0)