[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser
From: |
Taylor Simpson |
Subject: |
RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser |
Date: |
Tue, 7 Sep 2021 18:08:48 +0000 |
> -----Original Message-----
> From: Alessandro Di Federico <ale.qemu@rev.ng>
> Sent: Tuesday, July 20, 2021 7:30 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson <tsimpson@quicinc.com>; Brian Cain
> <bcain@quicinc.com>; babush@rev.ng; nizzo@rev.ng;
> richard.henderson@linaro.org; Alessandro Di Federico <ale@rev.ng>
> Subject: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser
>
> From: Paolo Montesel <babush@rev.ng>
>
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> Signed-off-by: Paolo Montesel <babush@rev.ng>
> ---
> target/hexagon/idef-parser/idef-parser.h | 258 +++++++
> target/hexagon/idef-parser/idef-parser.lex | 642 ++++++++++++++++++
> target/hexagon/meson.build | 4 +
> tests/docker/dockerfiles/alpine.docker | 1 +
> tests/docker/dockerfiles/centos8.docker | 1 +
> tests/docker/dockerfiles/debian-amd64.docker | 1 +
> tests/docker/dockerfiles/debian10.docker | 1 +
> .../dockerfiles/fedora-i386-cross.docker | 1 +
> .../dockerfiles/fedora-win32-cross.docker | 1 +
> .../dockerfiles/fedora-win64-cross.docker | 1 +
> tests/docker/dockerfiles/fedora.docker | 1 +
> tests/docker/dockerfiles/opensuse-leap.docker | 1 +
> tests/docker/dockerfiles/ubuntu.docker | 1 +
> tests/docker/dockerfiles/ubuntu1804.docker | 1 +
> tests/docker/dockerfiles/ubuntu2004.docker | 2 +
Here you are adding flex to several docker files, then in the next patch you
add bison. Create a single patch with the necessary changes to the docker
files together that is separate from the idef lexer for Hexagon.
> 15 files changed, 917 insertions(+)
> create mode 100644 target/hexagon/idef-parser/idef-parser.h
> create mode 100644 target/hexagon/idef-parser/idef-parser.lex
>
> diff --git a/target/hexagon/idef-parser/idef-parser.h b/target/hexagon/idef-
> parser/idef-parser.h
> new file mode 100644
> +/**
> + * Type of register, assigned to the HexReg.type field
> + */
> +typedef enum {GENERAL_PURPOSE, CONTROL, MODIFIER, DOTNEW}
> RegType;
Should be HexRegType to match the other names in this file (e.g., HexSignedness)
> +/**
> + * Semantic record of the PRE token, identifying a predicate
> + */
> +typedef struct HexPre {
> + char id; /**< Identifier of the predicate
> */
> +} HexPre;
Call this HexPred - HexPre sounds like something that comes before ...
> diff --git a/target/hexagon/idef-parser/idef-parser.lex
> b/target/hexagon/idef-parser/idef-parser.lex
> +REG_ID_32 e|s|d|t|u|v|x|y
> +REG_ID_64 ee|ss|dd|tt|uu|vv|xx|yy
> +SYS_ID_32 s|d
> +SYS_ID_64 ss|dd
> +LOWER_PRE d|s|t|u|v|e|x|x
Call this PRED_ID to be consistent with REG_ID_32, etc
> +"R"{REG_ID_32}"V" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = GENERAL_PURPOSE;
> + yylval->rvalue.reg.id = yytext[1];
> + yylval->rvalue.reg.bit_width = 32;
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.is_dotnew = false;
> + yylval->rvalue.signedness = SIGNED;
> + return REG; }
> +"in R"{REG_ID_32}"V" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = GENERAL_PURPOSE;
> + yylval->rvalue.reg.id = yytext[4];
> + yylval->rvalue.reg.bit_width = 32;
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.is_dotnew = false;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
Are you dropping the "in" here? This looks the same as the "R"(REG_ID_32}"V"
case.
Ditto for all the ones below ...
> +"in R"{REG_ID_64}"V" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = GENERAL_PURPOSE;
> + yylval->rvalue.reg.id = yytext[4];
> + yylval->rvalue.reg.bit_width = 64;
> + yylval->rvalue.bit_width = 64;
> + yylval->rvalue.is_dotnew = false;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
> +"in N"{REG_ID_32}"N" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = DOTNEW;
> + yylval->rvalue.reg.id = yytext[4];
> + yylval->rvalue.reg.bit_width = 32;
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.is_dotnew = true;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
> +"in N"{REG_ID_64}"N" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = DOTNEW;
> + yylval->rvalue.reg.id = yytext[4];
> + yylval->rvalue.reg.bit_width = 64;
> + yylval->rvalue.bit_width = 64;
> + yylval->rvalue.is_dotnew = true;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
> +"in P"{LOWER_PRE}"V" {
> + yylval->rvalue.type = PREDICATE;
> + yylval->rvalue.pre.id = yytext[4];
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.is_dotnew = false;
> + yylval->rvalue.signedness = SIGNED;
> + return RPRE; }
> +"in P"{LOWER_PRE}"N" {
> + yylval->rvalue.type = PREDICATE;
> + yylval->rvalue.pre.id = yytext[4];
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.is_dotnew = true;
> + yylval->rvalue.signedness = SIGNED;
> + return RPRE; }
> +"in MuV" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = MODIFIER;
> + yylval->rvalue.reg.id = 'u';
> + yylval->rvalue.reg.bit_width = 32;
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
> +"in C"{REG_ID_32}"V" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = CONTROL;
> + yylval->rvalue.reg.id = yytext[4];
> + yylval->rvalue.reg.bit_width = 32;
> + yylval->rvalue.bit_width = 32;
> + yylval->rvalue.is_dotnew = false;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
> +"in C"{REG_ID_64}"V" {
> + yylval->rvalue.type = REGISTER_ARG;
> + yylval->rvalue.reg.type = CONTROL;
> + yylval->rvalue.reg.id = yytext[4];
> + yylval->rvalue.reg.bit_width = 64;
> + yylval->rvalue.bit_width = 64;
> + yylval->rvalue.is_dotnew = false;
> + yylval->rvalue.signedness = SIGNED;
> + return RREG; }
> +"fGEN_TCG_"{INST_NAME}"(" { return FWRAP; }
When would one of these need to be parsed?
> +"fNEWREG" |
> +"fCAST4s" { yylval->cast.bit_width = 32;
> + yylval->cast.signedness = SIGNED;
> + return CAST; }
This doesn't look right - is fNEWREG the same as fCAST4s?
> +"fNEWREG_ST" |
> +"fIMMEXT" |
> +"fMUST_IMMEXT" |
> +"fCAST2_2s" |
> +"fCAST2_2u" |
> +"fCAST4_4s" |
> +"fCAST8_8s" |
> +"fZE8_16" |
> +"fSE8_16" |
> +"fZE16_32" |
> +"fSE16_32" |
> +"fZE32_64" |
> +"fPASS" |
> +"fECHO" { return IDENTITY; }
The fCAST, fZE, and fSE macros are not the same as identity.
> +"fCONSTLL" { return CONSTLL; }
> +"fCONSTULL" { return CONSTULL; }
These can just be converts.
> +"fHINTJR(RsV)" { /* Emit no token */ }
Put this in the list of IDENTITY above
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser,
Taylor Simpson <=