Skip to content

Commit f56e179

Browse files
jgu222sys_zuul
authored andcommitted
If a BB is both the target of backward goto and the fall-thru BB of another
backward goto BB, the join was incorrectly inserted into the head of the second loop. As this is not expected, the join in the loop head was set to a wrong JIP. (Simply put, two back-to-back loops will need an empty BB in between.) The fix is to pre-process CFG so that an empty BB is inserted to break this case. For example, B0 : (P) goto B0 B1 : (P1) goto B1 changed to B0 : (P) goto B0 newBB: B1 : (P1) goto B1 Change-Id: Ib3c32eead4770b676f09907e7a9ab074665ad35d
1 parent 1c2397e commit f56e179

File tree

1 file changed

+54
-21
lines changed

1 file changed

+54
-21
lines changed

visa/CFGStructurizer.cpp

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -901,16 +901,29 @@ ANode *ANode::getChildANode(ANode *parent)
901901

902902
// Preprocess CFG so that the structurizer can be simpler without considering
903903
/// those corner cases. Currently, it does:
904-
// *) avoid sharing target label among forward gotos and backward gotos
904+
// 1) avoid sharing target label among forward gotos and backward gotos
905905
// For example,
906906
// goto L
907907
// L:
908908
// goto L
909-
// to
909+
//
910+
// changed to
910911
// goto L0
911912
// L0:
912913
// L1:
913914
// goto L1
915+
// 2) avoid a fall-thru BB of a backward goto BB is the target BB of another
916+
// backward goto (two back-to-back loops). For example,
917+
// L0:
918+
// (p0) goto L0
919+
// L1 :
920+
// (p1) goto L1
921+
// changed to
922+
// L0:
923+
// (p0) goto L0
924+
// L :
925+
// L1 :
926+
// (p1) goto L1
914927
void CFGStructurizer::preProcess()
915928
{
916929
bool CFGChanged = false;
@@ -926,6 +939,7 @@ void CFGStructurizer::preProcess()
926939
// Check if this B is a successor of both backward and forward branches.
927940
bool isForwardTarget = false;
928941
bool isBackwardTarget = false;
942+
bool isFallThruOfBackwardGotoBB = false;
929943
BB_LIST_ITER IT, IE;
930944
for (IT = B->Preds.begin(), IE = B->Preds.end(); IT != IE; IT++)
931945
{
@@ -937,18 +951,30 @@ void CFGStructurizer::preProcess()
937951
}
938952
// P is a BB before B
939953
G4_INST *gotoInst = getGotoInst(B);
954+
G4_INST* gotoInstP = getGotoInst(P);
940955
if (gotoInst && P->getPhysicalSucc() != B)
941956
{
942957
isForwardTarget = true;
943958
}
959+
else if (gotoInstP
960+
&& P->getPhysicalSucc() == B
961+
&& gotoInstP->asCFInst()->isBackward())
962+
{
963+
isFallThruOfBackwardGotoBB = true;
964+
}
944965
}
945966

946-
if (!isBackwardTarget || !isForwardTarget)
967+
if ( !(isBackwardTarget && isForwardTarget) // case 1
968+
&& !(isBackwardTarget && isFallThruOfBackwardGotoBB)) // case 2
947969
{
970+
// not candidate
948971
continue;
949972
}
950973

951-
// "B" is the target of both forward and backward branching.
974+
// "B" is the target of both forward and backward branching, or is the
975+
// target of a backward branching and also the fall-thru of another
976+
// backward goto BB.
977+
//
952978
// Create an empty BB right before "B", and adjust all forward
953979
// branching to this new BB.
954980
G4_BB *newBB = createBBWithLabel();
@@ -967,7 +993,7 @@ void CFGStructurizer::preProcess()
967993
// keep the backward branch unchanged.
968994
continue;
969995
}
970-
// P->B changed to P->newBB
996+
// forward branching/fall-thru P->B is changed to P->newBB
971997
BBListReplace(P->Succs, B, newBB);
972998
newBB->Preds.push_back(P);
973999

@@ -1015,9 +1041,12 @@ void CFGStructurizer::init()
10151041
// (Note that removeUnreachableBlocks() right before invoking this
10161042
// pass will reassign block IDs.)
10171043

1018-
// First, preprocess CFG so that no forward gotos and backward gotos
1019-
// share the same label (all forward goto can share the same label,
1020-
// so can all backward gotos).
1044+
// First, preprocess CFG so that the structurizer does not need to handle corner cases.
1045+
// Currently, it does:
1046+
// 1. no forward gotos and backward gotos share the same label (but allow that
1047+
// all forward gotos can share the same label, so can all backward gotos);
1048+
// 2. No fall-thru BB of a backward goto BB is also the target BB of another
1049+
// backward goto.
10211050
preProcess();
10221051

10231052
BBs = &(CFG->getBBList());
@@ -2890,19 +2919,23 @@ void CFGStructurizer::setJoinJIP(G4_BB* joinBB, G4_BB* jipBB)
28902919
void CFGStructurizer::convertChildren(ANodeHG *nodehg, G4_BB *nextJoinBB)
28912920
{
28922921
// JIP setting of Join instruction:
2893-
// to set JIP correctly, we use activeJoinBBs that keeps a list
2894-
// of join BBs in the increasing order of BBs (in terms of BB layout
2895-
// order). The join BB is the BB with join instruction as its first
2896-
// instruction. With this list, a join's jip will be its next BB
2897-
// in activeJoinBBs.
2922+
// activeJoinBBs : a list of join BBs in the increasing order of BB layout,
2923+
// which are after the BB being processed currently.
2924+
//
2925+
// The join BB is the BB with join instruction as its first instruction.
2926+
// If the current BB is the first join BB (if exists) in acticveJoinBBs,
2927+
// its JIP will be set to the 2nd join in activeJoinBBs. Once its JIP is
2928+
// set, this BB is removed from activeJoinBBs.
28982929
//
2899-
// activeJoinBBs keeps a list of join BBs whose jips needs to be
2900-
// set. It works like this, for each child ANode, if its exit is
2901-
// join BB, add it into activeJoinBBs. We check if a BB is a join
2902-
// BB by checking if the BB has join instruction in it (we will
2903-
// convert the child first. If its exit is a join, a join will
2904-
// be insert in its exit BB, but this join's jip is not known until
2905-
// the next join is found). If so, add it into activeJoinBBs.
2930+
// activeJoinBBs keeps a list of join BBs whose jips needs to be set.
2931+
// It works like this: for each child ANode, if its exit BB is a join BB,
2932+
// add it into activeJoinBBs (we check if a BB is a join BB by checking
2933+
// if the BB has join instruction in it). As conversion is done bottom-up,
2934+
// all children must have been converted before their parents. This means
2935+
// that a join should be inserted in a child's exit BB if that child does
2936+
// need join. But that join's JIP is not known until the next join is found.
2937+
// Adding unknown-JIP join into activeJoinBBs so that it will get set when
2938+
// next next join is reached.
29062939
//
29072940
BB_LIST activeJoinBBs;
29082941
if (nextJoinBB)
@@ -2912,7 +2945,7 @@ void CFGStructurizer::convertChildren(ANodeHG *nodehg, G4_BB *nextJoinBB)
29122945
activeJoinBBs.push_back(nextJoinBB);
29132946
}
29142947

2915-
// For DO_WHILE, its end should have been processed in convertDoWhile()
2948+
// If nodehg is DO_WHILE, its end should have been processed in convertDoWhile()
29162949
// before invoking this function, hence, skip the end BB.
29172950
ANList::iterator II = nodehg->children.begin();
29182951
ANList::iterator IE = nodehg->children.end();

0 commit comments

Comments
 (0)