Skip to content

Commit 534cbd3

Browse files
jgu222igcbot
authored andcommitted
Avoid uniform phi in some case
Avoid the phi like: 'a = phi (x, b0) (x, b1)' by replacing 'a' with 'x', thus remove the phi. Note that lcssa phi is kept as WIA (uniform anaysis) needs it for a better and correct uniform analysis.
1 parent 3b10634 commit 534cbd3

File tree

3 files changed

+165
-0
lines changed

3 files changed

+165
-0
lines changed

IGC/Compiler/CISACodeGen/ShaderCodeGen.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,12 @@ static void AddAnalysisPasses(CodeGenContext& ctx, IGCPassManager& mpm)
333333
}
334334
mpm.add(createFixInvalidFuncNamePass());
335335

336+
//
337+
// Generally, passes that change IR should be prior to this place!
338+
//
339+
340+
// let CleanPHINode be right before Layout
341+
mpm.add(createCleanPHINodePass());
336342
// Let Layout be the last pass before Emit Pass
337343
mpm.add(new Layout());
338344

IGC/Compiler/CustomSafeOptPass.cpp

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5287,3 +5287,161 @@ bool LogicalAndToBranch::runOnFunction(Function& F)
52875287

52885288
return changed;
52895289
}
5290+
5291+
// clean PHINode does the following:
5292+
// given the following:
5293+
// a = phi (x, b0), (x, b1)
5294+
// this pass will replace 'a' with 'x', and as result, phi is removed.
5295+
//
5296+
// Special note:
5297+
// LCSSA PHINode has a single incoming value. Make sure it is not removed
5298+
// as WIA uses lcssa phi as a seperator between a uniform value inside loop
5299+
// and non-uniform value outside a loop. For example:
5300+
// B0:
5301+
// i = 0;
5302+
// Loop:
5303+
// i_0 = phi (0, B0) (t, Bn)
5304+
// .... <use i_0>
5305+
// if (divergent cond)
5306+
// Bi:
5307+
// goto out;
5308+
// Bn:
5309+
// t = i_0 + 1;
5310+
// if (t < N) goto Loop;
5311+
// goto output;
5312+
// out:
5313+
// i_1 = phi (i_0, Bi) <-- lcssa phi node
5314+
// ....
5315+
// output:
5316+
// Here, i_0 is uniform within the loop, but it is not outside loop as each WI will
5317+
// exit with different i, thus i_1 is non-uniform.
5318+
// If this lcssa phi would be removed, all uses of i_1 will be replaced with a uniform
5319+
// value i_0, which is wrong.
5320+
//
5321+
// This is needed to avoid generating the following code for which vISA cannot generate
5322+
// the correct code:
5323+
// i = 0; // i is uniform
5324+
// Loop:
5325+
// x = i + 1 // x is uniform
5326+
// B0 if (divergent-condition)
5327+
// <code1>
5328+
// B1 else
5329+
// z = array[i]
5330+
// B2 endif
5331+
// i = phi (x, B0), (x, B1)
5332+
// ......
5333+
// if (i < n) goto Loop
5334+
//
5335+
// Generated code (visa) (phi becomes mov in its incoming BBs).
5336+
//
5337+
// i = 0; // i is uniform
5338+
// Loop:
5339+
// (W) x = i + 1 // x is uniform, NoMask
5340+
// B0 if (divergent-condition)
5341+
// <code1>
5342+
// (W) i = x // noMask
5343+
// B1 else
5344+
// z = array[i]
5345+
// (W) i = x // noMask
5346+
// B2 endif
5347+
// ......
5348+
// if (i < n) goto Loop
5349+
//
5350+
// In the 1st iteration, 'z' should be array[0]. Assume 'if' is divergent, thus both B0 and B1
5351+
// blocks will be executed. As result, the value of 'i' after B0 will be x, which is 1. And 'z'
5352+
// will take array[1], which is wrong (correct one is array[0]).
5353+
//
5354+
// This case happens if phi is uniform, which means all phis' incoming values are identical
5355+
// and uniform (current hehavior of WIAnalysis). Note that the identical values means this phi
5356+
// is no longer needed. Once such a phi is removed, we will never generate code like one shown
5357+
// above and thus, no wrong code will be generated from visa.
5358+
//
5359+
// This pass will be invoked at place close to the Emit pass, where WIAnalysis will be invoked,
5360+
// so that IR between this pass and WIAnalysis stays the same, at least no new PHINodes like this
5361+
// will be generated.
5362+
//
5363+
namespace {
5364+
class CleanPHINode : public FunctionPass
5365+
{
5366+
public:
5367+
static char ID;
5368+
CleanPHINode();
5369+
5370+
StringRef getPassName() const override { return "CleanPhINode"; }
5371+
5372+
bool runOnFunction(Function& F) override;
5373+
};
5374+
}
5375+
5376+
#undef PASS_FLAG
5377+
#undef PASS_DESCRIPTION
5378+
#undef PASS_CFG_ONLY
5379+
#undef PASS_ANALYSIS
5380+
#define PASS_FLAG "igc-cleanphinode"
5381+
#define PASS_DESCRIPTION "Clean up PHINode"
5382+
#define PASS_CFG_ONLY false
5383+
#define PASS_ANALYSIS false
5384+
IGC_INITIALIZE_PASS_BEGIN(CleanPHINode, PASS_FLAG, PASS_DESCRIPTION, PASS_CFG_ONLY, PASS_ANALYSIS)
5385+
IGC_INITIALIZE_PASS_END(CleanPHINode, PASS_FLAG, PASS_DESCRIPTION, PASS_CFG_ONLY, PASS_ANALYSIS)
5386+
5387+
5388+
char CleanPHINode::ID = 0;
5389+
FunctionPass* IGC::createCleanPHINodePass()
5390+
{
5391+
return new CleanPHINode();
5392+
}
5393+
5394+
CleanPHINode::CleanPHINode() : FunctionPass(ID)
5395+
{
5396+
initializeCleanPHINodePass(*PassRegistry::getPassRegistry());
5397+
}
5398+
5399+
bool CleanPHINode::runOnFunction(Function& F)
5400+
{
5401+
auto isLCSSAPHINode = [](PHINode* PHI) { return (PHI->getNumIncomingValues() == 1); };
5402+
5403+
bool changed = false;
5404+
for (auto BI = F.begin(), BE = F.end(); BI != BE; ++BI)
5405+
{
5406+
BasicBlock* BB = &*BI;
5407+
auto II = BB->begin();
5408+
auto IE = BB->end();
5409+
while (II != IE)
5410+
{
5411+
auto currII = II;
5412+
++II;
5413+
PHINode* PHI = dyn_cast<PHINode>(currII);
5414+
if (PHI == nullptr)
5415+
{
5416+
// proceed to the next BB
5417+
break;
5418+
}
5419+
if (isLCSSAPHINode(PHI))
5420+
{
5421+
// Keep LCSSA PHI as uniform analysis needs it.
5422+
continue;
5423+
}
5424+
5425+
if (PHI->getNumIncomingValues() > 0) // sanity
5426+
{
5427+
Value* sameVal = PHI->getIncomingValue(0);
5428+
bool isAllSame = true;
5429+
for (int i = 1, sz = (int)PHI->getNumIncomingValues(); i < sz; ++i)
5430+
{
5431+
if (sameVal != PHI->getIncomingValue(i))
5432+
{
5433+
isAllSame = false;
5434+
break;
5435+
}
5436+
}
5437+
if (isAllSame)
5438+
{
5439+
PHI->replaceAllUsesWith(sameVal);
5440+
PHI->eraseFromParent();
5441+
changed = true;
5442+
}
5443+
}
5444+
}
5445+
}
5446+
return changed;
5447+
}

IGC/Compiler/CustomSafeOptPass.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,5 +245,6 @@ namespace IGC
245245
llvm::FunctionPass* createBlendToDiscardPass();
246246
llvm::FunctionPass* createMarkReadOnlyLoadPass();
247247
llvm::FunctionPass* createLogicalAndToBranchPass();
248+
llvm::FunctionPass* createCleanPHINodePass();
248249

249250
} // namespace IGC

0 commit comments

Comments
 (0)