Skip to content

Commit 5d1f530

Browse files
DianaChenigcbot
DianaChen
authored and
igcbot
committed
IGA refoctoring and code cleanup
Refactoring KernelParser to add helper function. Also some minor code cleanup and indent fix.
1 parent 4e4db7e commit 5d1f530

File tree

15 files changed

+110
-62
lines changed

15 files changed

+110
-62
lines changed

‎visa/BinaryEncodingIGA.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,7 @@ void BinaryEncodingIGA::translateInstructionSrcs(
13331333
igaInst->setInidirectSource(
13341334
opIx,
13351335
srcMod,
1336+
iga::RegName::GRF_R, // set to GRF for indirect register access
13361337
regRef,
13371338
srcRegion->getAddrImm(),
13381339
region,

‎visa/iga/IGALibrary/Backend/GED/Decoder.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1327,13 +1327,13 @@ void Decoder::decodeSendSource0(Instruction *inst)
13271327
rgn = decodeSrcRegionVWH<SourceIndex::SRC0>();
13281328
}
13291329

1330-
inst->setDirectSource(
1331-
SourceIndex::SRC0,
1332-
SrcModifier::NONE,
1333-
dri.regName,
1334-
dri.regRef,
1335-
rgn,
1336-
dri.type);
1330+
inst->setDirectSource(
1331+
SourceIndex::SRC0,
1332+
SrcModifier::NONE,
1333+
dri.regName,
1334+
dri.regRef,
1335+
rgn,
1336+
dri.type);
13371337
}
13381338
}
13391339

@@ -1932,6 +1932,7 @@ void Decoder::decodeSourceBasicAlign1(
19321932
inst->setInidirectSource(
19331933
toSrcIxE,
19341934
srcMod,
1935+
RegName::GRF_R, // set to GRF for indirect register access
19351936
a0,
19361937
addrImm,
19371938
decRgn,
@@ -2031,7 +2032,7 @@ void Decoder::decodeSourceBasicAlign16(
20312032
int32_t addrImm = decodeSrcAddrImm<S>();
20322033
RegRef indReg = {0, (uint8_t)subRegNum};
20332034
inst->setInidirectSource(
2034-
toSrcIx, srcMod, indReg,
2035+
toSrcIx, srcMod, RegName::GRF_R, indReg,
20352036
(int16_t)addrImm, Region::SRC110, decodeSrcType<S>());
20362037
} else {
20372038
// == GED_ADDR_MODE_INVALID

‎visa/iga/IGALibrary/Backend/GED/Encoder.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,7 +1435,7 @@ void Encoder::encodeSendDestination(const Operand& dst)
14351435
void Encoder::encodeSendSource0(const Operand& src)
14361436
{
14371437
if (m_model.supportsUnarySend()) {
1438-
switch( src.getKind() )
1438+
switch(src.getKind())
14391439
{
14401440
case Operand::Kind::DIRECT:
14411441
GED_ENCODE(Src0AddrMode, GED_ADDR_MODE_Direct);
@@ -1468,15 +1468,17 @@ void Encoder::encodeSendSource0(const Operand& src)
14681468
}
14691469
else if (src.getKind() == Operand::Kind::INDIRECT)
14701470
{
1471-
GED_ENCODE(Src0DataType, lowerDataType(t));
1472-
GED_ENCODE(Src0AddrSubRegNum, src.getIndAddrReg().subRegNum);
1473-
// For platform >= XeHPC, the ImmAddr is represented in Word Offset in bianry,
1474-
// platform < XeHPC, the ImmAddr is represented in Byte Offset in bianry
1475-
// And for all platforms, the ImmAddr is represented in Byet Offset in assembly
1476-
if (platform() >= Platform::XE_HPC) {
1477-
GED_ENCODE(Src0AddrImm, src.getIndImmAddr() / 2);
1478-
} else {
1479-
GED_ENCODE(Src0AddrImm, src.getIndImmAddr());
1471+
{
1472+
GED_ENCODE(Src0DataType, lowerDataType(t));
1473+
GED_ENCODE(Src0AddrSubRegNum, src.getIndAddrReg().subRegNum);
1474+
// For platform >= XeHPC, the ImmAddr is represented in Word Offset in bianry,
1475+
// platform < XeHPC, the ImmAddr is represented in Byte Offset in bianry
1476+
// And for all platforms, the ImmAddr is represented in Byet Offset in assembly
1477+
if (platform() >= Platform::XE_HPC) {
1478+
GED_ENCODE(Src0AddrImm, src.getIndImmAddr() / 2);
1479+
} else {
1480+
GED_ENCODE(Src0AddrImm, src.getIndImmAddr());
1481+
}
14801482
}
14811483
}
14821484
}

‎visa/iga/IGALibrary/Frontend/Formatter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,7 @@ class Formatter : public BasicFormatter
819819
emitAnsi(ANSI_REGISTER_GRF, "r");
820820
emit("[");
821821

822-
formatScalarRegRead(RegName::ARF_A, op.getIndAddrReg());
822+
formatScalarRegRead(RegName::ARF_A, op.getIndAddrReg());
823823

824824
if (op.getIndImmAddr() != 0) {
825825
int16_t val = op.getIndImmAddr();

‎visa/iga/IGALibrary/Frontend/KernelParser.cpp

Lines changed: 59 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,24 @@ class KernelParser : GenParser
815815
int m_sendSrcLens[2]; // send message lengths
816816
Loc m_sendSrcLenLocs[2]; // locations so we can referee
817817
bool m_implicitExBSO; // send src1 suffixed with length implies exBSO, but ensure the inst opt is given
818+
819+
private:
820+
// A helper function to add ARF_NULL src to given src operand
821+
void addNullSrc(int srcOpIx, bool isImmOrLbl) {
822+
// For instructions those have implicit types, match the type to
823+
// the expected one. Otherwise, ARF_NULL operand should have Type::INVALID
824+
Type type = Type::INVALID;
825+
m_opSpec->implicitSrcTypeVal(srcOpIx, isImmOrLbl, type);
826+
m_builder.InstSrcOpRegDirect(
827+
srcOpIx,
828+
m_srcLocs[srcOpIx],
829+
SrcModifier::NONE,
830+
RegName::ARF_NULL,
831+
REGREF_ZERO_ZERO,
832+
Region::SRC010,
833+
type);
834+
}
835+
818836
public:
819837
KernelParser(
820838
const Model &model,
@@ -1993,9 +2011,9 @@ class KernelParser : GenParser
19932011
// e.g. [a0.4, 16]
19942012
// or [a0.4 + 16]
19952013
// or [a0.4 - 16]
1996-
void ParseIndOpArgs(RegRef &addrRegRef, int &addrOff) {
2014+
void ParseIndOpArgs(RegRef &addrRegRef, int &addrOff, RegName& regName) {
19972015
ConsumeOrFail(LBRACK, "expected [");
1998-
if (!ParseAddrRegRefOpt(addrRegRef)) {
2016+
if (!ParseAddrRegRefOpt(addrRegRef, regName)) {
19992017
FailT("expected address subregister");
20002018
}
20012019
Loc addrOffLoc = NextLoc();
@@ -2037,7 +2055,8 @@ class KernelParser : GenParser
20372055
// [a0.4,16]
20382056
int addrOff;
20392057
RegRef addrRegRef;
2040-
ParseIndOpArgs(addrRegRef, addrOff);
2058+
RegName regName = RegName::INVALID;
2059+
ParseIndOpArgs(addrRegRef, addrOff, regName);
20412060
addrOff += baseAddr;
20422061
//
20432062
// <1>
@@ -2080,14 +2099,18 @@ class KernelParser : GenParser
20802099

20812100

20822101
// E.g. 3 in "a0.3"
2083-
bool ParseAddrRegRefOpt(RegRef& addrReg) {
2102+
bool ParseAddrRegRefOpt(RegRef& addrReg, RegName& regName) {
20842103
const RegInfo *ri;
20852104
int regNum;
2105+
regName = RegName::INVALID;
20862106
if (!ConsumeReg(ri, regNum)) {
20872107
return false;
20882108
}
2089-
if (ri->regName != RegName::ARF_A && regNum != 0) {
2090-
FailT("expected a0");
2109+
regName = ri->regName;
2110+
if (regNum != 0 ||
2111+
(ri->regName != RegName::ARF_A
2112+
)) {
2113+
FailT("expected address register for indirect access (a0)");
20912114
}
20922115
if (!Consume(DOT)) {
20932116
FailT("expected .");
@@ -2261,7 +2284,8 @@ class KernelParser : GenParser
22612284
// [a0.4 + 16]
22622285
int addrOff;
22632286
RegRef addrRegRef;
2264-
ParseIndOpArgs(addrRegRef, addrOff);
2287+
RegName regName = RegName::INVALID;
2288+
ParseIndOpArgs(addrRegRef, addrOff, regName);
22652289
addrOff += baseOff;
22662290

22672291
// regioning... <V;W,H> or <V,H>
@@ -2270,8 +2294,9 @@ class KernelParser : GenParser
22702294
// :t
22712295
Type sty = ParseSrcOpTypeWithDefault(srcOpIx, true);
22722296

2297+
IGA_ASSERT(regName != RegName::INVALID, "SrcOpInd must not have INVALID register name");
22732298
m_builder.InstSrcOpRegIndirect(
2274-
srcOpIx, opStart, srcMods, addrRegRef, addrOff, rgn, sty);
2299+
srcOpIx, opStart, srcMods, regName, addrRegRef, addrOff, rgn, sty);
22752300
}
22762301

22772302

@@ -2916,7 +2941,6 @@ class KernelParser : GenParser
29162941
void ParseSendSrc1OpWithOptLen(int &src1Len) {
29172942
m_srcLocs[1] = NextLoc();
29182943
src1Len = -1;
2919-
29202944
const Token regnameTk = Next();
29212945
const RegInfo *regInfo;
29222946
int regNum;
@@ -3201,7 +3225,9 @@ class KernelParser : GenParser
32013225
void ParseSendDescsWithOptSrc1Len(int src1Length) {
32023226
const Loc exDescLoc = NextLoc();
32033227
SendDesc exDesc;
3204-
if (ParseAddrRegRefOpt(exDesc.reg)) { // ExDesc is register
3228+
RegName regName = RegName::INVALID;
3229+
if (ParseAddrRegRefOpt(exDesc.reg, regName)) { // ExDesc is register
3230+
IGA_ASSERT(regName == RegName::ARF_A, "Indirect send exDesc must be ARF_A");
32053231
exDesc.type = SendDesc::Kind::REG32A;
32063232
if (src1Length >= 0) {
32073233
m_implicitExBSO = true;
@@ -3275,7 +3301,8 @@ class KernelParser : GenParser
32753301
//
32763302
const Loc descLoc = NextLoc();
32773303
SendDesc desc;
3278-
if (ParseAddrRegRefOpt(desc.reg)) {
3304+
if (ParseAddrRegRefOpt(desc.reg, regName)) {
3305+
IGA_ASSERT(regName == RegName::ARF_A, "Indirect send desc must be ARF_A");
32793306
desc.type = SendDesc::Kind::REG32A;
32803307
} else {
32813308
// constant integral expression
@@ -3302,7 +3329,9 @@ class KernelParser : GenParser
33023329

33033330
SendDesc ParseDesc(const char *which) {
33043331
SendDesc sd;
3305-
if (ParseAddrRegRefOpt(sd.reg)) { // ExDesc is register
3332+
RegName regName = RegName::INVALID;
3333+
if (ParseAddrRegRefOpt(sd.reg, regName)) { // ExDesc is register
3334+
IGA_ASSERT(regName == RegName::ARF_A, "Indirect send Desc must be ARF_A");
33063335
sd.type = SendDesc::Kind::REG32A;
33073336

33083337
} else { // ExDesc is imm
@@ -3329,7 +3358,9 @@ class KernelParser : GenParser
33293358

33303359
const Loc exDescLoc = NextLoc();
33313360
SendDesc exDesc;
3332-
if (ParseAddrRegRefOpt(exDesc.reg)) {
3361+
RegName regName = RegName::INVALID;
3362+
if (ParseAddrRegRefOpt(exDesc.reg, regName)) {
3363+
IGA_ASSERT(regName == RegName::ARF_A, "Indirect send exDesc must be ARF_A");
33333364
exDesc.type = SendDesc::Kind::REG32A;
33343365
if (platform() < Platform::XE)
33353366
m_builder.InstSubfunction(SFID::A0REG);
@@ -3363,7 +3394,8 @@ class KernelParser : GenParser
33633394

33643395
const Loc descLoc = NextLoc();
33653396
SendDesc desc;
3366-
if (ParseAddrRegRefOpt(desc.reg)) {
3397+
if (ParseAddrRegRefOpt(desc.reg, regName)) {
3398+
IGA_ASSERT(regName == RegName::ARF_A, "Indirect send Desc must be ARF_A");
33673399
desc.type = SendDesc::Kind::REG32A;
33683400
} else {
33693401
// constant integral expression
@@ -3406,7 +3438,8 @@ class KernelParser : GenParser
34063438
// TODO: sink this into the instop parsing once we remerge
34073439
// that with KernelParser... (after we rip out the legacy ld/st tables)
34083440
bool src1LengthSuffixSet = m_sendSrcLens[1] != -1;
3409-
if (instOpts.contains(InstOpt::EXBSO) && !src1LengthSuffixSet) {
3441+
if (instOpts.contains(InstOpt::EXBSO) && !src1LengthSuffixSet
3442+
) {
34103443
// GOOD: send ... r10:2 a0.# ... {ExBSO}
34113444
// ERROR: send ... r10 a0.# ... {ExBSO}
34123445
// Src1.Length comes from EU bits a0.# holds 26b offset
@@ -3433,7 +3466,9 @@ class KernelParser : GenParser
34333466
}
34343467

34353468
m_builder.InstOptsAdd(instOpts);
3436-
if (m_implicitExBSO && !instOpts.contains(InstOpt::EXBSO)) {
3469+
3470+
if (m_implicitExBSO && !instOpts.contains(InstOpt::EXBSO)
3471+
) {
34373472
WarningAtT(m_mnemonicLoc, "send src1 length implicitly added "
34383473
"(include {ExBSO})");
34393474
}
@@ -3621,11 +3656,11 @@ class KernelParser : GenParser
36213656
};
36223657
//
36233658
bool parsedNamedPipe =
3624-
tryParsePipe("F", SWSB::DistType::REG_DIST_FLOAT)
3625-
|| tryParsePipe("I", SWSB::DistType::REG_DIST_INT)
3626-
|| tryParsePipe("L", SWSB::DistType::REG_DIST_LONG)
3627-
|| tryParsePipe("A", SWSB::DistType::REG_DIST_ALL)
3628-
|| tryParsePipe("M", SWSB::DistType::REG_DIST_MATH);
3659+
tryParsePipe("F", SWSB::DistType::REG_DIST_FLOAT) ||
3660+
tryParsePipe("I", SWSB::DistType::REG_DIST_INT) ||
3661+
tryParsePipe("L", SWSB::DistType::REG_DIST_LONG) ||
3662+
tryParsePipe("A", SWSB::DistType::REG_DIST_ALL) ||
3663+
tryParsePipe("M", SWSB::DistType::REG_DIST_MATH);
36293664
if (parsedNamedPipe && m_model.supportsHwDeps()) {
36303665
FailAtT(loc,
36313666
"software dependencies not supported on this platform");
@@ -4214,7 +4249,9 @@ bool KernelParser::ParseLdStInst()
42144249
auto parseA0RegOrImm = [&] () {
42154250
SendDesc surf;
42164251
ConsumeOrFail(LBRACK, "expected [");
4217-
if (ParseAddrRegRefOpt(surf.reg)) {
4252+
RegName regName = RegName::INVALID;
4253+
if (ParseAddrRegRefOpt(surf.reg, regName)) {
4254+
IGA_ASSERT(regName == RegName::ARF_A, "Indirect send Desc must be ARF_A");
42184255
// surface is a register
42194256
surf.type = SendDesc::Kind::REG32A;
42204257
if (surf.reg.subRegNum & 1) {

‎visa/iga/IGALibrary/IR/InstBuilder.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ class InstBuilder {
460460
inst->setInidirectSource(
461461
opIx,
462462
src.regOpSrcMod,
463+
src.regOpName,
463464
src.regOpReg,
464465
src.regOpIndOff,
465466
src.regOpRgn,
@@ -753,6 +754,7 @@ class InstBuilder {
753754
int srcOpIx, // index of the current source operand
754755
const Loc &loc,
755756
const SrcModifier &srcMod, // source modifiers on this operand
757+
RegName regName,
756758
RegRef addrReg,
757759
int addrOff, // e.g. 16 in r[a0.3,16] (0 if absent)
758760
Region rgn,

‎visa/iga/IGALibrary/IR/Instruction.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,14 @@ void Instruction::setMacroSource(
104104
void Instruction::setInidirectSource(
105105
SourceIndex srcIx,
106106
SrcModifier srcMod,
107+
RegName regName,
107108
RegRef reg,
108109
int16_t immediateOffset,
109110
Region rgn,
110111
Type type)
111112
{
112113
unsigned ix = static_cast<unsigned>(srcIx);
113-
m_srcs[ix].setInidirectSource(srcMod, reg, immediateOffset, rgn, type);
114+
m_srcs[ix].setInidirectSource(srcMod, regName, reg, immediateOffset, rgn, type);
114115
}
115116

116117

@@ -184,6 +185,7 @@ bool Instruction::isMovWithLabel() const {
184185
getSource(0).getKind() == Operand::Kind::LABEL);
185186
}
186187

188+
187189
void Instruction::validate() const
188190
{
189191
iga::SanityCheckIR(*this);

‎visa/iga/IGALibrary/IR/Instruction.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ namespace iga
123123
void setInidirectSource(
124124
SourceIndex srcIx,
125125
SrcModifier srcMod,
126+
RegName regName,
126127
RegRef reg,
127128
int16_t m_immOffset,
128129
Region rgn,
@@ -249,6 +250,7 @@ namespace iga
249250

250251
bool isMovWithLabel() const;
251252

253+
252254
SWSB::InstType getSWSBInstType(SWSB_ENCODE_MODE mode) const;
253255

254256
void validate() const; // asserts on malformed IR

‎visa/iga/IGALibrary/IR/Operand.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ void Operand::setImmediateSource(
8181

8282
void Operand::setInidirectSource(
8383
SrcModifier srcMod,
84+
RegName regName,
8485
const RegRef &reg,
8586
int16_t immediateOffset,
8687
const Region &rgn,
@@ -92,7 +93,7 @@ void Operand::setInidirectSource(
9293
m_regOpSrcMod = srcMod;
9394
m_regOpReg = reg;
9495
m_regOpRgn = rgn;
95-
m_regOpName = RegName::GRF_R;
96+
m_regOpName = regName;
9697
m_regOpIndOff = immediateOffset;
9798
m_type = type;
9899
}

‎visa/iga/IGALibrary/IR/Operand.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ class Operand {
181181
// re-initializes this operand as an indirect register operand
182182
void setInidirectSource(
183183
SrcModifier srcMod,
184+
RegName regName,
184185
const RegRef &reg,
185186
int16_t addrImmOff,
186187
const Region &rgn,

0 commit comments

Comments
 (0)