RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser

From: Taylor Simpson <tsimpson@quicinc.com>
To: "anjo@rev.ng" <anjo@rev.ng>,
	Alessandro Di Federico <ale.qemu@rev.ng>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "babush@rev.ng" <babush@rev.ng>, Brian Cain <bcain@quicinc.com>,
	"richard.henderson@linaro.org" <richard.henderson@linaro.org>,
	"nizzo@rev.ng" <nizzo@rev.ng>,
	Alessandro Di Federico <ale@rev.ng>
Subject: RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser
Date: Thu, 28 Oct 2021 15:49:48 +0000	[thread overview]
Message-ID: <SN4PR0201MB8808233526630C34C93976F8DE869@SN4PR0201MB8808.namprd02.prod.outlook.com> (raw)
In-Reply-To: <49378670-138d-563b-9071-3fae6c95f265@rev.ng>

> From: Anton Johansson <anjo@rev.ng> 
> Sent: Monday, October 18, 2021 6:37 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; Alessandro Di Federico <ale.qemu@rev.ng>; qemu-devel@nongnu.org
> Cc: Brian Cain <bcain@quicinc.com>; babush@rev.ng; nizzo@rev.ng; richard.henderson@linaro.org; Alessandro Di Federico <ale@rev.ng>
> Subject: Re: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser
> 
> On 9/7/21 18:08, Taylor Simpson wrote:
> +"fNEWREG"                |
> +"fCAST4s"                { yylval->cast.bit_width = 32;
> +                           yylval->cast.signedness = SIGNED;
> +                           return CAST; }
> This doesn't look right - is fNEWREG the same as fCAST4s?
> We followed the definition of fNEWREG in macros.h where it is given as
>   #define fNEWREG(VAL) ((uint32_t) (VAL))

Well, that's different from fCAST4s.  In particular, one is signed and one is unsigned.

> +"fCONSTLL"               { return CONSTLL; }
> +"fCONSTULL"              { return CONSTULL; }
> These can just be converts.
> What is meant by "converts" here? 

Type conversion to int64_t/uint64_t.

> +"fHINTJR(RsV)"           { /* Emit no token */ }
> Put this in the list of IDENTITY above
> Same as for fNEWREG. We followed the definition in macros.h as 
>   #define fHINTJR(TARGET) { /* Not modelled in qemu */ } where it no-ops.

OK, as long as it is no-op.

Thanks,
Taylor