Skip to content

Commit 3a41c2a

Browse files
weiyu-chensys_zuul
weiyu-chen
authored and
sys_zuul
committed
(last try) Make regOff and subregOff for srcRegRegion const. Fix another case where indirect source was not handled correctly.
Change-Id: I2192031df0ddff69ef3f857454ab5047f23ad82b
1 parent e8de761 commit 3a41c2a

File tree

10 files changed

+94
-113
lines changed

10 files changed

+94
-113
lines changed

‎visa/BuildIR.h

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,6 +1345,48 @@ class IR_Builder {
13451345
return rgn;
13461346
}
13471347

1348+
G4_SrcRegRegion* createSrcWithNewRegOff(G4_SrcRegRegion* old, short newRegOff)
1349+
{
1350+
if (old->getRegAccess() == Direct)
1351+
{
1352+
return createSrcRegRegion(old->getModifier(), Direct, old->getBase(), newRegOff,
1353+
old->getSubRegOff(), old->getRegion(), old->getType(), old->getAccRegSel());
1354+
}
1355+
else
1356+
{
1357+
return createIndirectSrc(old->getModifier(), old->getBase(), newRegOff, old->getSubRegOff(),
1358+
old->getRegion(), old->getType(), old->getAddrImm());
1359+
}
1360+
}
1361+
1362+
G4_SrcRegRegion* createSrcWithNewSubRegOff(G4_SrcRegRegion* old, short newSubRegOff)
1363+
{
1364+
if (old->getRegAccess() == Direct)
1365+
{
1366+
return createSrcRegRegion(old->getModifier(), old->getRegAccess(), old->getBase(), old->getRegOff(),
1367+
newSubRegOff, old->getRegion(), old->getType(), old->getAccRegSel());
1368+
}
1369+
else
1370+
{
1371+
return createIndirectSrc(old->getModifier(), old->getBase(), old->getRegOff(), newSubRegOff,
1372+
old->getRegion(), old->getType(), old->getAddrImm());
1373+
}
1374+
}
1375+
1376+
G4_SrcRegRegion* createSrcWithNewBase(G4_SrcRegRegion* old, G4_VarBase* newBase)
1377+
{
1378+
if (old->getRegAccess() == Direct)
1379+
{
1380+
return createSrcRegRegion(old->getModifier(), Direct, newBase, old->getRegOff(),
1381+
old->getSubRegOff(), old->getRegion(), old->getType(), old->getAccRegSel());
1382+
}
1383+
else
1384+
{
1385+
return createIndirectSrc(old->getModifier(), newBase, old->getRegOff(), old->getSubRegOff(),
1386+
old->getRegion(), old->getType(), old->getAddrImm());
1387+
}
1388+
}
1389+
13481390
G4_SrcRegRegion* createIndirectSrc(G4_SrcModifier m,
13491391
G4_VarBase* b,
13501392
short roff,

‎visa/Gen4_IR.cpp

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3957,12 +3957,9 @@ void G4_Areg::emit(std::ostream& output, bool symbolreg)
39573957
// initial all values idential to rgn's
39583958
//
39593959
G4_SrcRegRegion::G4_SrcRegRegion(G4_SrcRegRegion &rgn)
3960-
: G4_Operand(G4_Operand::srcRegRegion)
3960+
: G4_Operand(G4_Operand::srcRegRegion), acc(rgn.acc), regOff(rgn.regOff), subRegOff(rgn.subRegOff)
39613961
{
39623962
base = rgn.base;
3963-
acc = rgn.acc;
3964-
regOff = rgn.regOff;
3965-
subRegOff = rgn.subRegOff;
39663963
mod = rgn.mod;
39673964
immAddrOff = rgn.immAddrOff;
39683965
desc = rgn.desc;
@@ -3973,6 +3970,7 @@ G4_SrcRegRegion::G4_SrcRegRegion(G4_SrcRegRegion &rgn)
39733970
*sw1 = *sw2;
39743971
accRegSel = rgn.accRegSel;
39753972

3973+
// FIXME: it's rather suspicious that we are copying internal fields this way
39763974
bitVec[0] = rgn.bitVec[0];
39773975
bitVec[1] = rgn.bitVec[1];
39783976

@@ -3982,31 +3980,7 @@ G4_SrcRegRegion::G4_SrcRegRegion(G4_SrcRegRegion &rgn)
39823980
byteOffset = rgn.byteOffset;
39833981
rightBoundSet = rgn.rightBoundSet;
39843982
}
3985-
//
3986-
// Initialize all values idential to rgn's, except for the base operand.
3987-
// Caller is responsible for allocating base operand and making sure it doesn't
3988-
// mess up the operands' hash table.
3989-
//
3990-
G4_SrcRegRegion::G4_SrcRegRegion(G4_SrcRegRegion &rgn, G4_VarBase *new_base)
3991-
: G4_Operand(G4_Operand::srcRegRegion)
3992-
{
3993-
acc = rgn.acc;
3994-
regOff = rgn.regOff;
3995-
subRegOff = rgn.subRegOff;
3996-
mod = rgn.mod;
3997-
immAddrOff = rgn.immAddrOff;
3998-
desc = rgn.desc;
3999-
type = rgn.type;
4000-
// copy swizzle value
4001-
char *sw1 = swizzle, *sw2 = rgn.swizzle;
4002-
while (*sw2) *sw1++ = *sw2++;
4003-
*sw1 = *sw2;
4004-
4005-
base = new_base;
40063983

4007-
computeLeftBound();
4008-
rightBoundSet = false;
4009-
}
40103984
//
40113985
// return true if rng and this have the same reg region
40123986
//

‎visa/Gen4_IR.hpp

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2827,10 +2827,10 @@ namespace vISA
28272827
char swizzle[max_swizzle]; // this should only be set in binary encoding
28282828

28292829
G4_SrcModifier mod;
2830-
G4_RegAccess acc; // direct, indirect GenReg or indirect MsgReg
2830+
const G4_RegAccess acc;
28312831
const RegionDesc *desc;
2832-
short regOff; // base+regOff is the starting register of the region
2833-
short subRegOff; // sub reg offset related to the regVar in "base"
2832+
const short regOff; // base+regOff is the starting register of the region
2833+
const short subRegOff; // sub reg offset related to the regVar in "base"
28342834
short immAddrOff; // imm addr offset
28352835

28362836
G4_SrcRegRegion(G4_SrcModifier m,
@@ -2852,9 +2852,10 @@ namespace vISA
28522852
right_bound = 0;
28532853
}
28542854

2855+
void setSrcBitVec(uint8_t exec_size);
2856+
28552857
public:
28562858
G4_SrcRegRegion(G4_SrcRegRegion& rgn);
2857-
G4_SrcRegRegion(G4_SrcRegRegion& rgn, G4_VarBase* new_base);
28582859
void *operator new(size_t sz, Mem_Manager& m) {return m.alloc(sz);}
28592860

28602861
bool operator==(const G4_SrcRegRegion &other)
@@ -2870,28 +2871,8 @@ namespace vISA
28702871
}
28712872

28722873
void computeLeftBound();
2873-
void setSrcBitVec(uint8_t exec_size);
28742874
short getRegOff() const { return regOff; }
28752875
short getSubRegOff() const { return subRegOff; }
2876-
void setSubRegOff(short off)
2877-
{
2878-
if (subRegOff != off)
2879-
{
2880-
subRegOff = off;
2881-
computeLeftBound();
2882-
unsetRightBound();
2883-
}
2884-
}
2885-
2886-
void setRegOff(short off)
2887-
{
2888-
if (regOff != off)
2889-
{
2890-
regOff = off;
2891-
unsetRightBound();
2892-
computeLeftBound();
2893-
}
2894-
}
28952876

28962877
const char* getSwizzle() const { return swizzle; }
28972878
G4_SrcModifier getModifier() const { return mod; }
@@ -2945,6 +2926,7 @@ namespace vISA
29452926

29462927
void setType(G4_Type ty)
29472928
{
2929+
// FIXME: we should forbid setType() where ty has a different size than old type
29482930
bool recomputeLeftBound = false;
29492931

29502932
if (G4_Type_Table[type].byteSize != G4_Type_Table[ty].byteSize)
@@ -2958,12 +2940,6 @@ namespace vISA
29582940
if (recomputeLeftBound)
29592941
{
29602942
computeLeftBound();
2961-
2962-
if (getInst())
2963-
{
2964-
getInst()->computeLeftBoundForImplAcc((G4_Operand*) getInst()->getImplAccDst());
2965-
getInst()->computeLeftBoundForImplAcc(getInst()->getImplAccSrc());
2966-
}
29672943
}
29682944
}
29692945

‎visa/GraphColor.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7896,8 +7896,8 @@ void VarSplit::insertMovesToTemp(IR_Builder& builder, G4_Declare* oldDcl, G4_Ope
78967896
{
78977897
unsigned maskFlag = (inst->getOption() & 0xFFF010C);
78987898
G4_DstRegRegion* dst = builder.Create_Dst_Opnd_From_Dcl(subDcl, 1);
7899-
G4_SrcRegRegion* src = builder.Create_Src_Opnd_From_Dcl(oldDcl, builder.getRegionStride1());
7900-
src->setRegOff((gra.getSubOffset(subDcl)) / G4_GRF_REG_NBYTES);
7899+
auto src = builder.createSrcRegRegion(Mod_src_undef, Direct, oldDcl->getRegVar(),
7900+
(gra.getSubOffset(subDcl)) / G4_GRF_REG_NBYTES, 0, builder.getRegionStride1(), oldDcl->getElemType());
79017901
G4_INST* splitInst = builder.createMov((uint8_t)subDcl->getTotalElems(), dst, src, maskFlag, false);
79027902
bb->insert(iter, splitInst);
79037903
}
@@ -7956,10 +7956,8 @@ void VarSplit::insertMovesFromTemp(G4_Kernel& kernel, G4_Declare* oldDcl, int in
79567956
bb->insert(instIter, movInst);
79577957
}
79587958
}
7959-
G4_SrcRegRegion* newSrc = kernel.fg.builder->Create_Src_Opnd_From_Dcl(newDcl, oldSrc->getRegion());
7960-
newSrc->setRegOff(0);
7961-
newSrc->setSubRegOff(oldSrc->getSubRegOff());
7962-
newSrc->setModifier(oldSrc->getModifier());
7959+
auto newSrc = kernel.fg.builder->createSrcRegRegion(oldSrc->getModifier(), Direct, newDcl->getRegVar(),
7960+
0, oldSrc->getSubRegOff(), oldSrc->getRegion(), newDcl->getElemType());
79637961
inst->setSrc(newSrc, pos);
79647962
}
79657963
else

‎visa/HWConformity.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6858,10 +6858,8 @@ static void expandPlaneMacro(IR_Builder& builder, INST_LIST_ITER it, G4_BB* bb,
68586858
G4_SrcRegRegion* srcR = builder.createSrcRegRegion(src0->getModifier(), Direct, src0->getBase(),
68596859
src0->getRegOff(), src0->getSubRegOff() + 3, builder.getRegionScalar(), src0->getType());
68606860

6861-
G4_SrcRegRegion* u = builder.duplicateOperand(src1);
6862-
u->setRegOff(u->getRegOff() + (secondHalf ? 2 : 0));
6863-
G4_SrcRegRegion* v = builder.duplicateOperand(src1);
6864-
v->setRegOff(v->getRegOff() + (secondHalf ? 3 : 1));
6861+
auto u = builder.createSrcWithNewRegOff(src1, src1->getRegOff() + (secondHalf ? 2 : 0));
6862+
auto v = builder.createSrcWithNewRegOff(src1, src1->getRegOff() + (secondHalf ? 3 : 1));
68656863

68666864
uint32_t options = inst->getOption();
68676865
if (inst->getExecSize() == 16)

‎visa/LocalRA.cpp

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2812,8 +2812,7 @@ bool LinearScan::allocateRegs(LocalLiveRange* lr, G4_BB* bb, IR_Builder& builder
28122812
if (totalElems == 32)
28132813
{
28142814
G4_DstRegRegion* dst = builder.createDstRegRegion(Direct, splitDcl->getRegVar(), 2, 0, 1, oldDcl->getElemType());
2815-
G4_SrcRegRegion* src = builder.Create_Src_Opnd_From_Dcl(oldDcl, builder.getRegionStride1());
2816-
src->setRegOff(2);
2815+
auto src = builder.createSrcRegRegion(Mod_src_undef, Direct, oldDcl->getRegVar(), 2, 0, builder.getRegionStride1(), oldDcl->getElemType());
28172816
G4_INST* splitInst2 = builder.createMov(16, dst, src, InstOpt_WriteEnable, false);
28182817
bb->insert(iter, splitInst2);
28192818
}
@@ -2843,8 +2842,7 @@ bool LinearScan::allocateRegs(LocalLiveRange* lr, G4_BB* bb, IR_Builder& builder
28432842
if (totalElems == 32)
28442843
{
28452844
G4_DstRegRegion* dst = builder.createDstRegRegion(Direct, newDcl->getRegVar(), 2, 0, 1, splitDcl->getElemType());
2846-
G4_SrcRegRegion* src = builder.Create_Src_Opnd_From_Dcl(splitDcl, builder.getRegionStride1());
2847-
src->setRegOff(2);
2845+
auto src = builder.createSrcRegRegion(Mod_src_undef, Direct, splitDcl->getRegVar(), 2, 0, builder.getRegionStride1(), splitDcl->getElemType());
28482846
G4_INST* movInst2 = builder.createMov(16, dst, src, InstOpt_WriteEnable, false);
28492847
bb->insert(iter, movInst2);
28502848
}
@@ -2861,11 +2859,8 @@ bool LinearScan::allocateRegs(LocalLiveRange* lr, G4_BB* bb, IR_Builder& builder
28612859
newSrcDcl->copyAlign(oldSrcDcl);
28622860
newSrcDcl->setAliasDeclare(aliasOldSrcDcl, oldSrcDcl->getAliasOffset());
28632861
}
2864-
G4_SrcRegRegion* newSrc = builder.Create_Src_Opnd_From_Dcl(newSrcDcl, oldSrc->getRegion());
2865-
newSrc->setRegOff(oldSrc->getRegOff());
2866-
newSrc->setSubRegOff(oldSrc->getSubRegOff());
2867-
newSrc->setModifier(oldSrc->getModifier());
2868-
newSrc->setType(oldSrc->getType());
2862+
auto newSrc = builder.createSrcRegRegion(oldSrc->getModifier(), Direct, newSrcDcl->getRegVar(),
2863+
oldSrc->getRegOff(), oldSrc->getSubRegOff(), oldSrc->getRegion(), oldSrc->getType());
28692864
useInst->setSrc(newSrc, pos);
28702865
while (aliasOldSrcDcl && aliasOldSrcDcl != oldDcl)
28712866
{
@@ -2894,11 +2889,8 @@ bool LinearScan::allocateRegs(LocalLiveRange* lr, G4_BB* bb, IR_Builder& builder
28942889
newSrcDcl->copyAlign(oldSrcDcl);
28952890
newSrcDcl->setAliasDeclare(aliasOldSrcDcl, oldSrcDcl->getAliasOffset());
28962891
}
2897-
G4_SrcRegRegion* newSrc = builder.Create_Src_Opnd_From_Dcl(newSrcDcl, oldSrc->getRegion());
2898-
newSrc->setRegOff(oldSrc->getRegOff());
2899-
newSrc->setSubRegOff(oldSrc->getSubRegOff());
2900-
newSrc->setModifier(oldSrc->getModifier());
2901-
newSrc->setType(oldSrc->getType());
2892+
auto newSrc = builder.createSrcRegRegion(oldSrc->getModifier(), Direct, newSrcDcl->getRegVar(),
2893+
oldSrc->getRegOff(), oldSrc->getSubRegOff(), oldSrc->getRegion(), oldSrc->getType());;
29022894
useInst->setSrc(newSrc, pos);
29032895
while (aliasOldSrcDcl && aliasOldSrcDcl != oldDcl)
29042896
{

‎visa/Optimizer.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3351,9 +3351,9 @@ void Optimizer::cselPeepHoleOpt()
33513351
useInst->setCondMod(builder.duplicateOperand(mod));
33523352
useInst->setPredicate(NULL);
33533353

3354-
G4_SrcRegRegion * opnd2 = useInst->getSrc(2)->asSrcRegRegion();
3354+
G4_SrcRegRegion* opnd2 = useInst->getSrc(2)->asSrcRegRegion();
33553355

3356-
if(!opnd2->isScalar() && inst->getExecSize() > useInst->getExecSize())
3356+
if (!opnd2->isScalar() && inst->getExecSize() > useInst->getExecSize())
33573357
{
33583358
//earlier check establishes that useInst mask is equivalent or subset
33593359
//sel instruction
@@ -3362,16 +3362,18 @@ void Optimizer::cselPeepHoleOpt()
33623362
cmp (16)
33633363
sel (8)
33643364
*/
3365-
if(useInst->getMaskOffset() != inst->getMaskOffset())
3365+
if (useInst->getMaskOffset() != inst->getMaskOffset())
33663366
{
3367-
//check elsewhere gurantees this is float.
3367+
//check elsewhere guarantees this is float.
33683368
G4_Type type = opnd2->getType();
3369-
unsigned short typeSize = (unsigned short) G4_Type_Table[type].byteSize;
3370-
unsigned offset = opnd2->getRegOff()* G4_GRF_REG_NBYTES + opnd2->getSubRegOff()*typeSize;
3369+
unsigned short typeSize = (unsigned short)G4_Type_Table[type].byteSize;
3370+
unsigned offset = opnd2->getRegOff() * G4_GRF_REG_NBYTES + opnd2->getSubRegOff() * typeSize;
33713371
offset += useInst->getExecSize() * src0Stride * typeSize;
33723372

3373-
opnd2->setRegOff(offset % G4_GRF_REG_NBYTES);
3374-
opnd2->setSubRegOff(offset / G4_GRF_REG_NBYTES);
3373+
auto newSrc2 = builder.createSrcRegRegion(opnd2->getModifier(), Direct, opnd2->getBase(),
3374+
offset / G4_GRF_REG_NBYTES, (offset % G4_GRF_REG_NBYTES) / typeSize, opnd2->getRegion(),
3375+
opnd2->getType());
3376+
useInst->setSrc(newSrc2, 2);
33753377
}
33763378
}
33773379
//
@@ -3457,12 +3459,14 @@ static void expandPseudoLogic(IR_Builder& builder,
34573459
// we have to use the upper flag bits (.1) instead
34583460
MUST_BE_TRUE(inst->getSrc(0)->isSrcRegRegion() && inst->getSrc(0)->isFlag(),
34593461
"expect src0 to be flag");
3460-
inst->getSrc(0)->asSrcRegRegion()->setSubRegOff(1);
3462+
auto newSrc0 = builder.createSrcWithNewSubRegOff(inst->getSrc(0)->asSrcRegRegion(), 1);
3463+
inst->setSrc(newSrc0, 0);
34613464
if (inst->getSrc(1) != nullptr)
34623465
{
34633466
MUST_BE_TRUE(inst->getSrc(1)->isSrcRegRegion() && inst->getSrc(1)->isFlag(),
34643467
"expect src1 to be flag");
3465-
inst->getSrc(1)->asSrcRegRegion()->setSubRegOff(1);
3468+
auto newSrc1 = builder.createSrcWithNewSubRegOff(inst->getSrc(1)->asSrcRegRegion(), 1);
3469+
inst->setSrc(newSrc1, 1);
34663470
}
34673471
inst->getDst()->setSubRegOff(1);
34683472
}
@@ -4080,7 +4084,8 @@ bool Optimizer::foldPseudoNot(G4_BB* bb, INST_LIST_ITER& iter)
40804084
{
40814085
// Fold upper bits
40824086
assert(notInst->getExecSize() == 16);
4083-
origUse->setSubRegOff(1);
4087+
origUse = builder.createSrcWithNewSubRegOff(origUse, 1);
4088+
notInst->setSrc(origUse, 0);
40844089
}
40854090
for (auto uses = notInst->use_begin(), uend = notInst->use_end(); uses != uend; ++uses)
40864091
{

‎visa/SpillCode.cpp

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -371,8 +371,7 @@ void SpillManager::replaceSpilledSrc(G4_BB* bb,
371371
}
372372
else
373373
{
374-
G4_SrcRegRegion rgn(*ss, spDcl->getRegVar()); // using spDcl as new base
375-
s = builder.createSrcRegRegion(rgn);
374+
s = builder.createSrcWithNewBase(ss, spDcl->getRegVar()); // using spDcl as new base
376375
}
377376
inst->setSrc(s,i);
378377
}
@@ -389,27 +388,27 @@ void SpillManager::replaceSpilledSrc(G4_BB* bb,
389388

390389
uint16_t num_reg = 1;
391390
//if access is VxH copy number of addresses based on execution size of instruction
392-
if(ss->getRegion()->isRegionWH())
391+
if (ss->getRegion()->isRegionWH())
393392
{
394393
num_reg = inst->getExecSize();
395394
}
396395

397396
G4_Declare* tmpDcl = NULL;
398397
bool match_found = false;
399398

400-
for(unsigned int j = 0; j < i; j++)
399+
for (unsigned int j = 0; j < i; j++)
401400
{
402-
G4_SrcRegRegion* analyzed_src = (G4_SrcRegRegion*) operands_analyzed[j];
403-
if( analyzed_src != NULL &&
401+
G4_SrcRegRegion* analyzed_src = (G4_SrcRegRegion*)operands_analyzed[j];
402+
if (analyzed_src != NULL &&
404403
analyzed_src->getBase()->asRegVar()->getDeclare() == ss->getBase()->asRegVar()->getDeclare() &&
405-
analyzed_src->getSubRegOff() == ss->getSubRegOff() )
404+
analyzed_src->getSubRegOff() == ss->getSubRegOff())
406405
{
407406
tmpDcl = declares_created[j];
408407
match_found = true;
409408
}
410409
}
411410

412-
if( !match_found )
411+
if (!match_found)
413412
{
414413
tmpDcl = createNewTempAddrDeclare(spDcl, num_reg);
415414
operands_analyzed[i] = ss;
@@ -424,13 +423,13 @@ void SpillManager::replaceSpilledSrc(G4_BB* bb,
424423
tmpDcl->getNumElems(), getGenxPlatform() >= GENX_CNL ? false : true);
425424
}
426425

427-
G4_SrcRegRegion rgn(*ss, tmpDcl->getRegVar()); // using tmpDcl as new base
428-
G4_SrcRegRegion* s = builder.createSrcRegRegion(rgn);
429-
s->setSubRegOff(0);
430-
inst->setSrc(s,i);
431-
if( !match_found )
426+
// create new src from the temp address variable, with offset 0
427+
auto s = builder.createIndirectSrc(ss->getModifier(), tmpDcl->getRegVar(), ss->getRegOff(), 0,
428+
ss->getRegion(), ss->getType(), ss->getAddrImm());
429+
inst->setSrc(s, i);
430+
if (!match_found)
432431
{
433-
pointsToAnalysis.insertAndMergeFilledAddr( ss->getBase()->asRegVar(), tmpDcl->getRegVar() );
432+
pointsToAnalysis.insertAndMergeFilledAddr(ss->getBase()->asRegVar(), tmpDcl->getRegVar());
434433
}
435434
}
436435
else

0 commit comments

Comments
 (0)