Skip to content

Commit 88f4362

Browse files
trbauerZuul
authored andcommitted
1. fixes defect where 32word and 64word in align directives failed parse
2. protects vISA generated variables from conflicting with vISA keywords. - vISA reserved words are suffixed with _ (iteratively). - we include tests against all ops and subops Change-Id: I8378a92e2db0e04a563f6db8746c1d85abf179e8
1 parent 383d331 commit 88f4362

File tree

6 files changed

+73
-12
lines changed

6 files changed

+73
-12
lines changed

visa/CISA.l

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ v_type[ ]*=[ ]*T {
732732
return T_CLASS;
733733
}
734734

735-
type[ ]*=[ ]*(ud|d|uw|w|ub|b|df|f|bool|uq|q|UD|D|UW|W|UB|B|DF|F|Bool|BOOL|UQ|Q|hf|HF) {
735+
type[ ]*=[ ]*(ud|d|uw|w|ub|b|df|f|bool|uq|q|UD|D|UW|W|UB|B|DF|F|Bool|BOOL|UQ|Q|hf|HF) {
736736
TRACE("\n** TYPE ");
737737
CISAlval.type = str2type(yytext, yyleng);
738738
return DECL_DATA_TYPE;
@@ -743,7 +743,20 @@ type[ ]*=[ ]*(ud|d|uw|w|ub|b|df|f|bool|uq|q|UD|D|UW|W|UB|B|DF|F|Bool|BOOL|UQ|Q|h
743743
TRACE("\n** AlignType - 2GRF ");
744744
CISAlval.align = ALIGN_2_GRF;
745745
// fprintf(stderr, "%s", "2GRF symbol is deprecated; please use GRFx2");
746-
return ALIGNTYPE_2GRF;
746+
return ALIGNTYPE_NONVAR_KEYWORD;
747+
}
748+
749+
32word {
750+
/* other cases are handled as VAR */
751+
TRACE("\n** AlignType - 32word ");
752+
CISAlval.align = ALIGN_32WORD;
753+
return ALIGNTYPE_NONVAR_KEYWORD;
754+
}
755+
64word {
756+
/* other cases are handled as VAR */
757+
TRACE("\n** AlignType - 64word ");
758+
CISAlval.align = ALIGN_64WORD;
759+
return ALIGNTYPE_NONVAR_KEYWORD;
747760
}
748761

749762
"(-)" {TRACE("\n** SRCMOD_NEG "); return SRCMOD_NEG;}

visa/CISA.y

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ VISA_RawOpnd* rawOperandArray[16];
356356
%token <string> RTWRITE_OPTION
357357

358358
%token <string> STRING_LITERAL
359-
%token <CISA_align> ALIGNTYPE_2GRF
359+
%token <CISA_align> ALIGNTYPE_NONVAR_KEYWORD
360360
%token <media_mode> MEDIA_MODE
361361
%token <oword_mod> OWORD_MODIFIER
362362
%token <s_channel> SAMPLER_CHANNEL
@@ -730,8 +730,8 @@ AlignType : /* empty */
730730
{
731731
$$ = ALIGN_BYTE;
732732
}
733-
| ALIGN ALIGNTYPE_2GRF {
734-
$$ = ALIGN_BYTE;
733+
| ALIGN ALIGNTYPE_NONVAR_KEYWORD {
734+
$$ = $2;
735735
}
736736
| ALIGN VAR /* e.g. byte, word, dword, qword, GRF, GRFx2 */
737737
{
@@ -2063,7 +2063,7 @@ static bool ParseAlign(CISA_IR_Builder* pCisaBuilder, const char *sym, VISA_Alig
20632063
value = ALIGN_OWORD;
20642064
} else if (strcmp(sym, "GRF") == 0) {
20652065
value = ALIGN_GRF;
2066-
} else if (strcmp(sym, "GRFx2") == 0) {
2066+
} else if (strcmp(sym, "GRFx2") == 0 || strcmp(sym, "2GRF") == 0) {
20672067
value = ALIGN_2_GRF;
20682068
} else if (strcmp(sym, "hword") == 0) {
20692069
value = ALIGN_HWORD;

visa/IsaDescription.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1998,8 +1998,7 @@ static const ISA_SubInst_Desc SVMSubOpcodeDesc[] =
19981998
};
19991999

20002000

2001-
static const ISA_SubInst_Desc *getSubInstTable(
2002-
uint8_t opcode, int &size)
2001+
const ISA_SubInst_Desc *getSubInstTable(uint8_t opcode, int &size)
20032002
{
20042003
const ISA_SubInst_Desc *table = nullptr;
20052004
switch (opcode)
@@ -2017,7 +2016,9 @@ static const ISA_SubInst_Desc *getSubInstTable(
20172016
size = sizeof(SVMSubOpcodeDesc)/sizeof(SVMSubOpcodeDesc[0]);
20182017
break;
20192018
default:
2020-
MUST_BE_TRUE(false, "instruction does not have sub opcode");
2019+
table = nullptr;
2020+
size = 0;
2021+
break;
20212022
}
20222023
return table;
20232024
}

visa/IsaDescription.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,11 @@ struct VISA_INST_Desc
236236
const ISA_SubInst_Desc& getSubInstDescByName(const char *symbol) const;
237237
};
238238

239+
// looks up a parent opcode and resolves the sub ops of that parent
240+
// returns nullptr and sets 'size' to 0 if an op doesn't have a subop.
241+
// e.g. ... = getSubInstTable(ISA_SVM, svmSubOps);
242+
const ISA_SubInst_Desc *getSubInstTable(uint8_t opcode, int &size);
243+
239244
enum SVMSubOpcode
240245
{
241246
SVM_BLOCK_LD = 0x1,

visa/VISAKernel.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ class VISAKernelImpl : public VISAFunction
135135
m_GenNamedVarMap.emplace_back();
136136

137137
createKernelAttributes();
138+
createReservedKeywordSet();
138139
}
139140

140141
void* alloc(size_t sz) { return m_mem.alloc(sz); }
@@ -852,6 +853,8 @@ class VISAKernelImpl : public VISAFunction
852853
void computeAndEmitDebugInfo(VISAKernelImplListTy& functions);
853854

854855
private:
856+
void createReservedKeywordSet();
857+
bool isReservedName(const std::string &nm) const;
855858
void ensureVariableNameUnique(const char *&varName);
856859
void generateVariableName(Common_ISA_Var_Class Ty, const char *&varName);
857860

@@ -1009,6 +1012,7 @@ class VISAKernelImpl : public VISAFunction
10091012
// TODO: this should be merged and re-worked to fit into the symbol table
10101013
// scheme
10111014
std::unordered_set<std::string> varNames;
1015+
std::unordered_set<std::string> reservedNames;
10121016

10131017
int m_vISAInstCount;
10141018
print_decl_index_t m_printDeclIndex;

visa/VISAKernelImpl.cpp

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -763,12 +763,47 @@ void VISAKernelImpl::createBindlessSampler()
763763
}
764764
}
765765

766+
void VISAKernelImpl::createReservedKeywordSet() {
767+
for (int i = 0; i < ISA_NUM_OPCODE; i++) {
768+
const VISA_INST_Desc &desc = CISA_INST_table[i];
769+
if (desc.name != nullptr)
770+
reservedNames.insert(desc.name);
771+
int subOpsLen = 0;
772+
const ISA_SubInst_Desc *subOps = getSubInstTable(desc.opcode, subOpsLen);
773+
if (subOps != nullptr) {
774+
// e.g. ops like ISA_SVM have a subtable of operations
775+
for (int si = 0; si < subOpsLen; si++) {
776+
// same tables have some padding and empty ops
777+
if (subOps[si].name != nullptr)
778+
reservedNames.insert(subOps[si].name);
779+
}
780+
}
781+
}
782+
783+
}
784+
785+
bool VISAKernelImpl::isReservedName(const std::string &nm) const {
786+
auto opItr = reservedNames.find(nm);
787+
return (opItr != reservedNames.end());
788+
}
789+
766790
void VISAKernelImpl::ensureVariableNameUnique(const char *&varName)
767791
{
768-
// escape the name to ensure it's a legal vISA identifier
792+
// legalize the LLVM name to vISA standards; some examples follow:
793+
// given ==> we fix it to this
794+
// 1. "0" ==> "_0" (LLVM name)
795+
// 2. "add.i.i" ==> "add_i_i" (LLVM compound name)
796+
// 3. "mul" ==> "mul_" (vISA keyword)
797+
// 4. suppose both variable "x" and "x0" exist
798+
// "x" ==> "x_1" (since "x0" already used)
799+
// "x0" ==> "x0_1" (it's a dumb suffixing strategy)
769800
std::stringstream escdName;
801+
802+
// step 1
770803
if (isdigit(varName[0]))
771804
escdName << '_';
805+
806+
// step 2
772807
for (size_t i = 0, slen = strlen(varName); i < slen; i++) {
773808
char c = varName[i];
774809
if (!isalnum(c)) {
@@ -777,6 +812,11 @@ void VISAKernelImpl::ensureVariableNameUnique(const char *&varName)
777812
escdName << c;
778813
}
779814

815+
// case 3: "mul" ==> "mul_"
816+
while (isReservedName(escdName.str()))
817+
escdName << '_';
818+
819+
// case 4: if "x" already exists, then use "x_#" where # is 0,1,..
780820
std::string varNameS = escdName.str();
781821
if (varNames.find(varNameS) != varNames.end()) {
782822
// not unqiue, add a counter until it is unique
@@ -787,14 +827,12 @@ void VISAKernelImpl::ensureVariableNameUnique(const char *&varName)
787827
varNameS = ss.str();
788828
} while (varNames.find(varNameS) != varNames.end());
789829
}
790-
791830
varNames.insert(varNameS);
792831

793832
char *buf = (char*)m_mem.alloc(varNameS.size() + 1);
794833
memcpy_s(buf, varNameS.size(), varNameS.c_str(), varNameS.size());
795834
buf[varNameS.size()] = 0;
796835
varName = buf;
797-
varNames.insert(varNameS);
798836
}
799837

800838
void VISAKernelImpl::generateVariableName(Common_ISA_Var_Class Ty, const char *&varName)

0 commit comments

Comments
 (0)