Re: [PATCH 0/3] yacc: compute the best type for the state number

[Akim Demaille]

> Le 5 oct. 2019 à 11:21, Paul Eggert <address@hidden> a écrit :
> 
> On 10/1/19 1:35 AM, Paul Eggert wrote:
>> Actually those arrays use int_fast16_t not int16_t, as C99/C11/C18 does not 
>> require support for int16_t. It could well be more efficient for them to use 
>> int_least16_t instead, for better caching
> 
> Attached is a proposed patch to implement this for Bison. This patch also 
> fixes some portability problems for odd machines like the TI TMS320C55x where 
> 'unsigned char', 'unsigned short', and 'unsigned' are all the same width 
> (which C allows),

Wow...  Why have they chosen this?  I guess the point is speed.

> and where picky -Wconversion compilers warn about assigning an unsigned char 
> value to an int variable.
> 
> Comments welcome; I haven't installed this.


> From 242504a103da5fc5cb868614a603710bad6a446a Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Sat, 5 Oct 2019 02:18:45 -0700
> Subject: [PATCH] =?UTF-8?q?Use=20=E2=80=9Cleast=E2=80=9D=20types=20for=20i?=
>  =?UTF-8?q?ntegers=20in=20Yacc=20tables?=
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> This changes the Yacc skeleton to use “least” integer types to
> keep tables smaller on some platforms, which should lessen cache
> pressure.  Since Bison uses the Yacc skeleton, it follows suit.

Maybe you want to extend the notes you added to NEWS.  Including
the bits about limits.h and stdint.h.  Maybe that should also make
its way into the doc, yet I wouldn't know where to write that.

> * data/skeletons/yacc.c (yytype_uint8, yytype_int8)
> (yytype_uint16, yytype_int16): If available, use GCC predefined
> macros __INT_MAX__ etc. to select a “least” type, as this avoids
> namespace hassles.  Otherwise, if available (because the relevant
> headers have been included) fall back on selecting a “least” type
> via the C99 macros INT_MAX, INT_LEAST8_MAX, etc.  Otherwise, fall
> further back on one of the builtin C99 types signed char, short,
> and int.  Make sure that any selected type promotes to int.
> Ignore any macros YYTYPE_INT16, YYTYPE_INT8, YYTYPE_UINT16,
> YYTYPE_UINT8 defined by the user.
> (yytype_uint8, yytype_uint16): Do not assume that unsigned char
> and unsigned short promote to int, as this isn’t true on some
> platforms (e.g., TI TMS320C55x).

Eventually, all this should move up into c.m4, and be applied to
glr.c too.  And see what to do about C++ parsers.

> * src/parse-gram.y: Include stdint.h, to help the yacc skeleton.
> (YYTYPE_INT16, YYTYPE_INT8, YYTYPE_UINT16, YYTYPE_UINT8):
> Remove, as these are no longer effective.

Good.


> +/* Narrow types that promote to a signed type and that can represent a
> +   signed or unsigned integer of at least N bits.  In tables they can
> +   save space and decrease cache pressure.  For best results, either
> +   use a compiler like GCC that predefines macros like __INT_MAX__, or
> +   include limits.h and stdint.h in your grammar prolog.  */
> +
> +#if defined __UINT_LEAST8_MAX__ && __UINT_LEAST8_MAX__ <= __INT_MAX__
> +typedef __UINT_LEAST8_TYPE__ yytype_uint8;
> +#elif defined UINT_LEAST8_MAX && defined INT_MAX && UINT_LEAST8_MAX <= 
> INT_MAX
> +typedef uint_least8_t yytype_uint8;
> +#elif defined UCHAR_MAX && UCHAR_MAX <= INT_MAX
>  typedef unsigned char yytype_uint8;
> +#else
> +typedef int yytype_uint8;
>  #endif

Wow!  Why do we fallback to int?  Is this part where unsigned int == unsigned 
short == unsigned char on a little number of architecture?  IIUC, you prefer to 
fall back to many bytes on all the architectures that don't define UCHAR_MAX 
rather than having warnings about storing an unsigned in an unsigned type?  I 
guess that will be frequent: users of other compilers than GCC who don't 
include limits.h.

I don't understand that.  I don't remember receiving bug reports for 
portability issues with "typedef unsigned char yytype_uint8".  The medicine 
seems worse than the disease to me.