Skip to content

Commit b2cc01d

Browse files
petechouigcbot
authored andcommitted
Reland "Fix the calculation of jitinfo's usesBarrier in finalizer."
Calculate the number of barriers when translating the vISA barrier instructions. For named barriers, derive the number from barrier ID instead of the number of send barrier instructions. For the legacy barrier, set it to 1 when there's a barrier instruction. Also when propagating the barrier information from callee to caller, change the caller's usesBarrier to callee's value if callee's value is bigger than caller's.
1 parent 9e87804 commit b2cc01d

File tree

4 files changed

+56
-14
lines changed

4 files changed

+56
-14
lines changed

visa/BuildCISAIRImpl.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,9 @@ static void retrieveBarrierInfoFromCallee(VISAKernelImpl* entry, std::set<VISAKe
12191219
// path? For zebin path, the barrier information of an indirect
12201220
// (external) function will be provided in the corresponding .ze_info
12211221
// field.
1222-
entry->getIRBuilder()->getJitInfo()->usesBarrier |= callee->getIRBuilder()->getJitInfo()->usesBarrier;
1222+
entry->getIRBuilder()->getJitInfo()->usesBarrier =
1223+
std::max(entry->getIRBuilder()->getJitInfo()->usesBarrier,
1224+
callee->getIRBuilder()->getJitInfo()->usesBarrier);
12231225
}
12241226
}
12251227

visa/Optimizer.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8212,11 +8212,10 @@ bool Optimizer::foldPseudoAndOr(G4_BB* bb, INST_LIST_ITER& ii)
82128212
}
82138213
}
82148214

8215-
// perform simple stat collection (e.g., numBarriers, numSends)
8215+
// perform simple stat collection (e.g., numSends)
82168216
// IR is not modified
82178217
void Optimizer::collectStats()
82188218
{
8219-
builder.getJitInfo()->usesBarrier = 0;
82208219
uint32_t numSends = 0;
82218220
for (auto bb : fg)
82228221
{
@@ -8225,14 +8224,6 @@ bool Optimizer::foldPseudoAndOr(G4_BB* bb, INST_LIST_ITER& ii)
82258224
if (inst->isSend())
82268225
{
82278226
numSends++;
8228-
if (inst->asSendInst()->getMsgDesc()->isBarrier())
8229-
{
8230-
// Propagate information about barriers presence back to IGC. It's safer to
8231-
// depend on vISA statistics as IGC is not able to detect barriers if they are
8232-
// used as a part of Inline vISA code.
8233-
// This information is used by legacy CMRT as well as OpenCL/L0 runtime.
8234-
builder.getJitInfo()->usesBarrier += 1;
8235-
}
82368227
}
82378228
}
82388229
}

visa/VisaToG4/TranslateSendSync.cpp

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ static void checkNamedBarrierSrc(G4_Operand* src, bool isBarrierId)
277277
if (isBarrierId)
278278
{
279279
uint32_t val = (uint32_t)src->asImm()->getInt();
280-
assert(val < 32 && "illegal named barrier id");
280+
assert(val < FINALIZER_INFO::kMaxNamedBarriers && "illegal named barrier id");
281281
}
282282
}
283283
else if (src->isSrcRegRegion())
@@ -291,12 +291,36 @@ static void checkNamedBarrierSrc(G4_Operand* src, bool isBarrierId)
291291
}
292292
}
293293

294+
static void updateNamedBarriersInJitInfo(FINALIZER_INFO* jitinfo, G4_Operand* barrierId)
295+
{
296+
if (!jitinfo)
297+
return;
298+
299+
if (barrierId->isImm())
300+
{
301+
// Update the number of named barriers to barrier id + 1 as the id is a
302+
// 0-based number.
303+
unsigned id = (unsigned)barrierId->asImm()->getInt();
304+
jitinfo->usesBarrier = std::max(jitinfo->usesBarrier, id + 1);
305+
}
306+
else
307+
{
308+
// In order to be safe, set the number of named barriers to the max
309+
// value allowed if the barrier id is unknown to vISA. We probably
310+
// won't see this in typical cases as the barrier id provided by users
311+
// like IGC should be an immediate value.
312+
jitinfo->usesBarrier = FINALIZER_INFO::kMaxNamedBarriers;
313+
}
314+
}
315+
294316
int IR_Builder::translateVISANamedBarrierWait(G4_Operand* barrierId)
295317
{
296318
TIME_SCOPE(VISA_BUILDER_IR_CONSTRUCTION);
297319

298320
checkNamedBarrierSrc(barrierId, true);
299321

322+
updateNamedBarriersInJitInfo(getJitInfo(), barrierId);
323+
300324
G4_Operand* barSrc = barrierId;
301325
if (barrierId->isSrcRegRegion()) {
302326
// sync can take only flag src
@@ -319,6 +343,8 @@ int IR_Builder::translateVISANamedBarrierSignal(G4_Operand* barrierId, G4_Operan
319343
checkNamedBarrierSrc(barrierId, true);
320344
checkNamedBarrierSrc(threadCount, false);
321345

346+
updateNamedBarriersInJitInfo(getJitInfo(), barrierId);
347+
322348
if (threadCount->isImm())
323349
{
324350
int numThreads = (int)threadCount->asImm()->getInt();
@@ -424,9 +450,21 @@ int IR_Builder::translateVISAWaitInst(G4_Operand* mask)
424450
return VISA_SUCCESS;
425451
}
426452

453+
static void updateBarrierInJitInfo(FINALIZER_INFO* jitinfo)
454+
{
455+
if (!jitinfo)
456+
return;
457+
458+
// For the legacy barrier, update usesBarrier to 1 when there's a
459+
// barrier.
460+
if (jitinfo->usesBarrier == 0)
461+
jitinfo->usesBarrier = 1;
462+
}
427463

428464
void IR_Builder::generateBarrierSend()
429465
{
466+
updateBarrierInJitInfo(getJitInfo());
467+
430468
if (hasUnifiedBarrier())
431469
{
432470
generateSingleBarrier();
@@ -478,6 +516,8 @@ void IR_Builder::generateBarrierSend()
478516

479517
void IR_Builder::generateBarrierWait()
480518
{
519+
updateBarrierInJitInfo(getJitInfo());
520+
481521
G4_Operand* waitSrc = nullptr;
482522
if (!hasUnifiedBarrier()) {
483523

visa/include/JitterDataStruct.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,17 @@ typedef struct _FINALIZER_INFO{
3939
unsigned numFlagSpillStore;
4040
unsigned numFlagSpillLoad;
4141

42-
// whether kernel uses a barrier
43-
unsigned usesBarrier;
42+
// Propagate information about barriers presence back to IGC. It's safer to
43+
// depend on vISA statistics as IGC is not able to detect barriers if they
44+
// are used as a part of Inline vISA code.
45+
// This information is used by legacy CMRT as well as OpenCL/L0 runtime.
46+
//
47+
// The number of named barriers; it also indicates whether kernel uses a
48+
// barrier.
49+
// TODO: Maybe consider to use a bitset to track the barrier IDs used, and
50+
// the number of bits set can be used to represent the number of barriers.
51+
unsigned usesBarrier = 0;
52+
static constexpr unsigned kMaxNamedBarriers = 32;
4453

4554
unsigned BBNum;
4655
VISA_BB_INFO* BBInfo;

0 commit comments

Comments
 (0)