Re: FW: [PATCH][STEP 2] ctype changes for removing locale structures

[Corinna Vinschen <vinschen at redhat dot com>]

On Feb  9 19:09, Jaap de Wolff wrote:
> Van: Jaap de Wolff [mailto:info@jasoon.nl] 
> > Van: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
> > On 02/09/2018 10:06 AM, Jaap de Wolff wrote:
> > > modify ctype not to use __locale_ctype_ptr (== __CTYPE_PTR) or 
> > > __locale_ctype_ptr_l To do this al inline functions are removed, and 
> > > the source implementations test hard coded values of the given 
> > > characters
> > >
> > >
> > > ================================== PATCH ================= diff --
> > git
> > > a/newlib/libc/ctype/isalnum.c b/newlib/libc/ctype/isalnum.c index
> > > d926f97b7..6d3864114 100644
> > > --- a/newlib/libc/ctype/isalnum.c
> > > +++ b/newlib/libc/ctype/isalnum.c
> > > @@ -46,5 +46,9 @@ No OS subroutines are required.
> > >   int
> > >   isalnum (int c)
> > >   {
> > > +#ifdef _REENT_SMALL
> > > +	return (isdigit(c) | islower(c) | isupper(c)); #else
> > >   	return(__CTYPE_PTR[c+1] & (_U|_L|_N));
> > > +#endif
> > >   }
> > I would think that we should be able to do better than this, as this 
> > method is taking a half step backwards for some of the functions, such 
> > as isalnum().  That is, it potentially loses the efficiency of doing 
> > one table lookup plus one OR, as it needs to do N table lookups and 
> > N+N-1 ORs (where N appears to be from 1-3).  I say "potentially" 
> > because maybe the compiler would end up reducing it to the
> > 1+1 case, but maybe not, too (depending on compiler, compiler 
> > 1+settings). Before
> > locale was added, isalnum(), for example, was
> > 
> > return(__ctype_ptr__[c+1] & (_U|_L));
> > 
> > I'm not sure what the right thing is now (still __ctype_ptr__?), but 
> > the same basic thing ought to work, knowing the right thing to put in.  
> > But for that matter, why not in the REENT_SMALL case simply redefine 
> > __CTYPE_PTR to point to the right thing?  Then all these other source 
> > file changes become unnecessary. It seems reasonable to expect this to 
> > work, and it would be a cleaner solution (since the is*.c files remain 
> > untouched, without #ifs).  The stated purpose of the patch is to stop 
> > ctype from using __locale_ctype_ptr, but ctype.h is where it is defined to be __CTYPE_PTR--so just define it differently.
> > 
> > Craig
> 
> Craig,
> In basic the intention of my patches is to make to memory footprint for embedded environment small.
> I talked about locale, and a part of the locale definition is a lookup table with the characteristics of each character.
> Of course it is possible to move the lookup table outside the locale, but then the lookup table still takes 257 bytes of memory.
> 
> In the beginning my intention was just to move the __global_locale table from ram to rom, and a reaction was that that was not enough when using embedded processors with 2K or even 1K of ROM and 128 bytes of RAM.
> 
> I do think that for the intended users of the REENT_SMALL computation speed (efficiency) is less important as memory.
> So I replaces all usage of a lookup table by a (kind of) computation.

I'm more with what Craig said.  A pointer to a character table has been
used since newlib started in the 90s.  We never had complaints for using
a single character table before.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: signature.asc
Description: PGP signature