Skip to content

Commit a824b25

Browse files
mshelegoigcbot
mshelego
authored and
igcbot
committed
[Autobackout][FuncReg]Revert of change: dba3ef6
Move loaded constants outside of loops GenXCategory pass loads constants and produced intructions are placed just before their users. If a user is inside a loop, then it results in immediate MOVs inside loop's body in vISA, because there is no LICM pass after GenXCategory. Loaded constants have to be moved just before the loop to avoid that
1 parent 5fa5b28 commit a824b25

File tree

4 files changed

+34
-190
lines changed

4 files changed

+34
-190
lines changed

‎IGC/VectorCompiler/lib/GenXCodeGen/GenXCategory.cpp

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ namespace {
172172
vc::KernelMetadata KM;
173173
GenXLiveness *Liveness = nullptr;
174174
DominatorTreeGroupWrapperPass *DTs = nullptr;
175-
LoopInfoGroupWrapperPass *LIs = nullptr;
176175
const GenXSubtarget *Subtarget = nullptr;
177176
const DataLayout *DL = nullptr;
178177
SmallVector<Instruction *, 8> ToErase;
@@ -391,7 +390,6 @@ namespace {
391390
INITIALIZE_PASS_BEGIN(GenXCategoryWrapper, "GenXCategoryWrapper",
392391
"GenXCategoryWrapper", false, false)
393392
INITIALIZE_PASS_DEPENDENCY(DominatorTreeGroupWrapperPassWrapper)
394-
INITIALIZE_PASS_DEPENDENCY(LoopInfoGroupWrapperPassWrapper);
395393
INITIALIZE_PASS_DEPENDENCY(GenXLivenessWrapper)
396394
INITIALIZE_PASS_END(GenXCategoryWrapper, "GenXCategoryWrapper",
397395
"GenXCategoryWrapper", false, false)
@@ -403,14 +401,12 @@ namespace {
403401

404402
void GenXCategory::getAnalysisUsage(AnalysisUsage &AU) {
405403
AU.addRequired<DominatorTreeGroupWrapperPass>();
406-
AU.addRequired<LoopInfoGroupWrapperPass>();
407404
AU.addRequired<GenXLiveness>();
408405
AU.addRequired<TargetPassConfig>();
409406
AU.addPreserved<GenXModule>();
410407
AU.addPreserved<GenXLiveness>();
411408
AU.addPreserved<FunctionGroupAnalysis>();
412409
AU.addPreserved<DominatorTreeGroupWrapperPass>();
413-
AU.addPreserved<LoopInfoGroupWrapperPass>();
414410
AU.setPreservesCFG();
415411
}
416412

@@ -422,7 +418,6 @@ bool GenXCategory::runOnFunctionGroup(FunctionGroup &FG)
422418
{
423419
KM = vc::KernelMetadata{FG.getHead()};
424420
DTs = &getAnalysis<DominatorTreeGroupWrapperPass>();
425-
LIs = &getAnalysis<LoopInfoGroupWrapperPass>();
426421
Liveness = &getAnalysis<GenXLiveness>();
427422
Subtarget = &getAnalysis<TargetPassConfig>()
428423
.getTM<GenXTargetMachine>()
@@ -534,50 +529,35 @@ static bool commonUpPredicate(BasicBlock *BB) {
534529
*/
535530
bool GenXCategory::processFunction(Function *F)
536531
{
537-
auto *LI = LIs->getLoopInfo(F);
538532
Func = F;
539533
// Before doing the category conversion, fix circular phis.
540534
Modified = fixCircularPhis(F);
541535
// Load constants in phi nodes.
542536
loadPhiConstants(*F, DTs->getDomTree(F), *Subtarget, *DL, false);
543537
// Process all instructions.
544-
DenseMap<Instruction *, SmallVector<Instruction *, 16>> MoveMap;
545538
for (po_iterator<BasicBlock *> i = po_begin(&Func->getEntryBlock()),
546539
e = po_end(&Func->getEntryBlock()); i != e; ++i) {
547540
// This loop scans the basic block backwards. If any code is inserted
548541
// before the current point, that code is scanned too.
549542
BasicBlock *BB = *i;
550-
// There is no LICM pass after GenXCategory, so we have to make sure,
551-
// that no constants are added inside a loop. They have to be moved
552-
// before the loop (or the outermost loop in case of a nested one)
553-
SmallVectorImpl<Instruction *> *AddedInsts = nullptr;
554-
if (auto *L = LI->getLoopFor(BB)) {
555-
while (L->getParentLoop())
556-
L = L->getParentLoop();
557-
if (auto *BeforeTheLoopBB = L->getLoopPredecessor())
558-
AddedInsts = &MoveMap[BeforeTheLoopBB->getTerminator()];
559-
}
560543
for (Instruction *Inst = &BB->back(); Inst;
561-
Inst = (Inst == &BB->front() ? nullptr : Inst->getPrevNode())) {
562-
Modified |= loadNonSimpleConstants(Inst, *Subtarget, *DL, AddedInsts);
563-
Modified |= loadConstants(Inst, *Subtarget, *DL, AddedInsts);
544+
Inst = (Inst == &BB->front() ? nullptr : Inst->getPrevNode())) {
545+
Modified |= loadNonSimpleConstants(Inst, *Subtarget, *DL, nullptr);
546+
Modified |= loadConstants(Inst, *Subtarget, *DL);
564547
if (!processValue(Inst))
565548
NoCategory.push_back(Inst);
566549
}
567550

568551
// This commons up constpred calls just loaded.
569552
Modified |= commonUpPredicate(BB);
553+
554+
// Erase instructions (and their live ranges) as requested by processValue.
555+
for (unsigned i = 0, e = ToErase.size(); i != e; ++i) {
556+
Liveness->eraseLiveRange(ToErase[i]);
557+
ToErase[i]->eraseFromParent();
558+
}
559+
ToErase.clear();
570560
}
571-
// Move instructions outside of loops
572-
for (auto &It : MoveMap)
573-
for (auto *InstToMove : It.second)
574-
InstToMove->moveBefore(It.first);
575-
// Erase instructions (and their live ranges) as requested by processValue.
576-
for (auto ToEraseInst : ToErase) {
577-
Liveness->eraseLiveRange(ToEraseInst);
578-
ToEraseInst->eraseFromParent();
579-
}
580-
ToErase.clear();
581561
// Process all args.
582562
for (auto fi = Func->arg_begin(), fe = Func->arg_end(); fi != fe; ++fi) {
583563
Value *V = &*fi;

‎IGC/VectorCompiler/lib/GenXCodeGen/GenXConstants.cpp

Lines changed: 22 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,7 @@ bool genx::loadConstantsForInlineAsm(
275275
* loadPhiConstants.
276276
*/
277277
bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
278-
const DataLayout &DL,
279-
SmallVectorImpl<Instruction *> *AddedInstructions) {
278+
const DataLayout &DL) {
280279
bool Modified = false;
281280
Use *U;
282281
if (isa<PHINode>(Inst))
@@ -298,9 +297,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
298297
return C1 && C1->isAllOnesValue();
299298
};
300299
if (!IsNot()) {
301-
Inst->setOperand(
302-
oi, ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
303-
.load(Inst));
300+
Inst->setOperand(oi, ConstantLoader(C, Subtarget, DL).load(Inst));
304301
Modified = true;
305302
}
306303
}
@@ -309,8 +306,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
309306
// select: disallow constant selector
310307
U = &Inst->getOperandUse(0);
311308
if (auto C = dyn_cast<Constant>(*U)) {
312-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
313-
.load(Inst);
309+
*U = ConstantLoader(C, Subtarget, DL).load(Inst);
314310
Modified = true;
315311
}
316312
return Modified;
@@ -320,8 +316,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
320316
// on element operand.
321317
U = &Inst->getOperandUse(1);
322318
if (auto C = dyn_cast<Constant>(*U)) {
323-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
324-
.load(Inst);
319+
*U = ConstantLoader(C, Subtarget, DL).load(Inst);
325320
Modified = true;
326321
}
327322
// Also disallow constant (other than undef) on old struct value operand.
@@ -336,9 +331,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
336331
// Conditional branch: disallow constant condition.
337332
if (Br->isConditional()) {
338333
if (auto C = dyn_cast<Constant>(Br->getCondition())) {
339-
Br->setCondition(
340-
ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
341-
.load(Br));
334+
Br->setCondition(ConstantLoader(C, Subtarget, DL).load(Br));
342335
Modified = true;
343336
}
344337
}
@@ -351,9 +344,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
351344
Ret->getFunction()->getLinkage() == GlobalValue::InternalLinkage) {
352345
if (auto C = dyn_cast<Constant>(Ret->getOperand(0))) {
353346
if (!C->getType()->isVoidTy() && !isa<UndefValue>(C)) {
354-
Ret->setOperand(
355-
0, ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
356-
.load(Ret));
347+
Ret->setOperand(0, ConstantLoader(C, Subtarget, DL).load(Ret));
357348
Modified = true;
358349
}
359350
}
@@ -364,7 +355,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
364355
if (!CI)
365356
return Modified;
366357
if (CI->isInlineAsm())
367-
return loadConstantsForInlineAsm(CI, Subtarget, DL, AddedInstructions);
358+
return loadConstantsForInlineAsm(CI, Subtarget, DL, nullptr);
368359
int IntrinsicID = vc::getAnyIntrinsicID(CI);
369360
switch (IntrinsicID) {
370361
case GenXIntrinsic::not_any_intrinsic:
@@ -380,8 +371,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
380371
U = &CI->getOperandUse(i);
381372
if (auto C = dyn_cast<Constant>(*U)) {
382373
if (!isa<UndefValue>(C)) {
383-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
384-
.loadBig(CI);
374+
*U = ConstantLoader(C, Subtarget, DL).loadBig(CI);
385375
Modified = true;
386376
}
387377
}
@@ -395,8 +385,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
395385
// abs modifier: disallow constant input.
396386
U = &CI->getOperandUse(0);
397387
if (auto C = dyn_cast<Constant>(*U)) {
398-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
399-
.load(CI);
388+
*U = ConstantLoader(C, Subtarget, DL).load(CI);
400389
Modified = true;
401390
}
402391
break;
@@ -406,8 +395,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
406395
// rdpredregion, any, all: disallow constant input
407396
U = &CI->getOperandUse(0);
408397
if (auto C = dyn_cast<Constant>(*U)) {
409-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
410-
.load(CI);
398+
*U = ConstantLoader(C, Subtarget, DL).load(CI);
411399
Modified = true;
412400
}
413401
break;
@@ -416,16 +404,14 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
416404
// rdregion: disallow constant input
417405
U = &CI->getOperandUse(0);
418406
if (auto C = dyn_cast<Constant>(*U)) {
419-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
420-
.loadBig(CI);
407+
*U = ConstantLoader(C, Subtarget, DL).loadBig(CI);
421408
Modified = true;
422409
}
423410
// Also disallow constant vector index (constant scalar OK).
424411
U = &CI->getOperandUse(GenXIntrinsic::GenXRegion::RdIndexOperandNum);
425412
if (auto C = dyn_cast<Constant>(*U)) {
426413
if (isa<VectorType>(C->getType())) {
427-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
428-
.load(CI);
414+
*U = ConstantLoader(C, Subtarget, DL).load(CI);
429415
Modified = true;
430416
}
431417
}
@@ -436,8 +422,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
436422
U = &CI->getOperandUse(0);
437423
if (auto C = dyn_cast<Constant>(*U)) {
438424
if (!isa<UndefValue>(C)) {
439-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
440-
.loadBig(CI);
425+
*U = ConstantLoader(C, Subtarget, DL).loadBig(CI);
441426
Modified = true;
442427
}
443428
}
@@ -448,26 +433,23 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
448433
U = &CI->getOperandUse(0);
449434
if (auto C = dyn_cast<Constant>(*U)) {
450435
if (!isa<UndefValue>(C)) {
451-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
452-
.loadBig(CI);
436+
*U = ConstantLoader(C, Subtarget, DL).loadBig(CI);
453437
Modified = true;
454438
}
455439
}
456440
// Also disallow constant vector index (constant scalar OK).
457441
U = &CI->getOperandUse(GenXIntrinsic::GenXRegion::WrIndexOperandNum);
458442
if (auto C = dyn_cast<Constant>(*U)) {
459443
if (isa<VectorType>(C->getType())) {
460-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
461-
.load(CI);
444+
*U = ConstantLoader(C, Subtarget, DL).load(CI);
462445
Modified = true;
463446
}
464447
}
465448
// Also disallow constant predicate unless all ones.
466449
U = &CI->getOperandUse(GenXIntrinsic::GenXRegion::PredicateOperandNum);
467450
if (auto C = dyn_cast<Constant>(*U)) {
468451
if (!C->isAllOnesValue()) {
469-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
470-
.load(CI);
452+
*U = ConstantLoader(C, Subtarget, DL).load(CI);
471453
Modified = true;
472454
}
473455
}
@@ -480,8 +462,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
480462
U = &CI->getOperandUse(2);
481463
if (auto C = dyn_cast<Constant>(*U)) {
482464
if (!C->isNullValue()) {
483-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
484-
.load(CI);
465+
*U = ConstantLoader(C, Subtarget, DL).load(CI);
485466
Modified = true;
486467
}
487468
}
@@ -533,8 +514,7 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
533514
continue;
534515
}
535516
// Operand is not allowed to be constant. Insert code to load it.
536-
*U = ConstantLoader(C, Subtarget, DL, nullptr, AddedInstructions)
537-
.loadBig(CI);
517+
*U = ConstantLoader(C, Subtarget, DL).loadBig(CI);
538518
Modified = true;
539519
}
540520
break;
@@ -1305,7 +1285,7 @@ Instruction *ConstantLoader::loadSplatConstant(Instruction *InsertPos) {
13051285
// Create <1 x T> constant and broadcast it through rdregion.
13061286
Constant *CV = ConstantVector::get(C1);
13071287
// Load that scalar constant first.
1308-
ConstantLoader L(CV, Subtarget, DL, nullptr, AddedInstructions);
1288+
ConstantLoader L(CV, Subtarget, DL);
13091289
Value *V = L.load(InsertPos);
13101290
// Broadcast through rdregion.
13111291
Instruction *Result = nullptr;
@@ -1415,7 +1395,7 @@ Instruction *ConstantLoader::loadNonPackedIntConst(Instruction *InsertBefore) {
14151395
std::min(PowerOf2Floor(NumElements - Idx), (uint64_t)ImmIntVec::Width);
14161396
Constant *SubC = getConstantSubvector(C, Idx, Size);
14171397
Value *SubV = SubC;
1418-
ConstantLoader SubLoader(SubC, Subtarget, DL, nullptr, AddedInstructions);
1398+
ConstantLoader SubLoader(SubC, Subtarget, DL);
14191399
SubV = SubLoader.load(InsertBefore);
14201400

14211401
Region R(C, &DL);
@@ -1461,8 +1441,7 @@ Instruction *ConstantLoader::loadPackedInt(Instruction *Inst) {
14611441
cast<ConstantInt>(PackedVals.back())->getSExtValue() <= 15);
14621442
}
14631443

1464-
ConstantLoader Packed(ConstantVector::get(PackedVals), Subtarget, DL, nullptr,
1465-
AddedInstructions);
1444+
ConstantLoader Packed(ConstantVector::get(PackedVals), Subtarget, DL);
14661445
auto *LoadPacked = Packed.loadNonPackedIntConst(Inst);
14671446
if (PackedIntScale != 1) {
14681447
auto *SplatVal =
@@ -1667,7 +1646,7 @@ Instruction *ConstantLoader::loadBig(Instruction *InsertBefore) {
16671646
// value with wrregion.
16681647
Constant *SubC = getConstantSubvector(C, Idx, Size);
16691648
Value *SubV = SubC;
1670-
ConstantLoader SubLoader(SubC, Subtarget, DL, nullptr, AddedInstructions);
1649+
ConstantLoader SubLoader(SubC, Subtarget, DL);
16711650
if (!SubLoader.isSimple())
16721651
SubV = SubLoader.loadNonSimple(InsertBefore);
16731652
Region R(C, &DL);

‎IGC/VectorCompiler/lib/GenXCodeGen/GenXConstants.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*========================== begin_copyright_notice ============================
22
3-
Copyright (C) 2020-2024 Intel Corporation
3+
Copyright (C) 2020-2023 Intel Corporation
44
55
SPDX-License-Identifier: MIT
66
@@ -154,8 +154,7 @@ bool loadConstantsForInlineAsm(
154154

155155
// Load constants used in an instruction.
156156
bool loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
157-
const DataLayout &DL,
158-
SmallVectorImpl<Instruction *> *AddedInstructions = nullptr);
157+
const DataLayout &DL);
159158

160159
// Load constants used in phi nodes in a function.
161160
bool loadPhiConstants(Function &F, DominatorTree *DT,

0 commit comments

Comments
 (0)