Re: [Nasm-devel] [PATCH 0/6] AVX-512: Bug fixes and additional features

[Song, J. K. <jin...@in...>]

> -----Original Message-----
> From: Cyrill Gorcunov [mailto:gor...@gm...]
> Sent: Thursday, August 22, 2013 8:45 AM
> To: Song, Jin Kyu
> Cc: nas...@li...; H. Peter Anvin
> Subject: Re: [Nasm-devel] [PATCH 0/6] AVX-512: Bug fixes and additional
> features
> 
> On Wed, Aug 21, 2013 at 07:29:07PM -0700, Jin Kyu Song wrote:
> > Please review these patches and pull if they look good.
> > git://repo.or.cz/nasm/avx512.git
> >
> > After running a test case, various issues were found. One major thing is
> > curly brace already used for grouping multi-line macro parameters.
> > An escape backward slash character '\' is added when braces are passed
> > as a part of enclosed parameter. The test asm file used here is also
> included.
> >
> > Patch "AVX-512: Add a test case for EVEX encoded instructions" is
> > relatively huge. So I did not attch that patch in this email. Please
> refer to
> >
> http://repo.or.cz/w/nasm/avx512.git/commitdiff/a4a573c47f3d9ddfd5c25218044
> 54327765f367e
> 
> Hi Jin, I've picked up all patches and pushed them on avx512 branch,
> thanks a lot,
> good job!
> 
> One question -- you use TOK_BRACE for both { and } terms, won't it be
> better to
> introduce two terms instead TOK_OPEN_BRACE and TOK_CLOSE_BRACE? How
> tokenizer
> will handle statements like
> 
> 	term \{ term \{
> 
> it will be treated as non-error case? (I must admit I didn't yet review
> the whole avx code :(

Hi Cyrill,

This case might be treated as an error in a parser not in a preprocessor if braces do not match even after expanding all macros. But this patch is for the multi-line macro preprocessing.

I used TOK_BRACE for the braces inside the parameter - "\{" or "\}". So they should be handled as a part of normal string without any special meaning. The reason why I added a new token type is tok_is_()/ tok_isnt_() macros check if it is TOK_OTHER or not. 
	#define tok_is_(x,v)    (tok_type_((x), TOK_OTHER) && !strcmp((x)->text,(v)))
	#define tok_isnt_(x,v)  ((x) && ((x)->type!=TOK_OTHER || strcmp((x)->text,(v))))

	"{" with TOK_OTHER : an opening brace of a parameter
	"{" with TOK_BRACE : same as any normal character. Originally it is "\{".

So a new token type could easily avoid this type checking while holding a curly brace as a string. I chose this way to minimize the change. Maybe I need to rename the new token type because people may think the name of TOK_BRACE means the brace actually enclosing the macro parameter.

At first I tried to change the parsing logic of preprocessor but that way led me to much bigger code change.

Thanks,
Jin