On Mon, 10 Jun 2019 17:09:10 -0500
Segher Boessenkool <segher@kernel.crashing.org> wrote:

> On Mon, Jun 10, 2019 at 08:58:00PM +0100, Jozef Lawrynowicz wrote:
> > On Mon, 10 Jun 2019 13:32:42 -0500
> > Segher Boessenkool <segher@kernel.crashing.org> wrote:  
> > > That is not a fix, that is sweeping the problem under the rug.
> > > 
> > > As a somewhat dirty hack I added
> > > 
> > > #if __MSP430X_LARGE__
> > > #undef __SIZE_TYPE__
> > > __extension__ typedef unsigned __int20 __SIZE_TYPE__;
> > > #endif
> > > 
> > > to the start of the installed stddef.h, and that fixes the problem fine,
> > > for correct programs that do not forget to include <stddef.h> (directly
> > > or indirectly), anyway.  
> 
> > But we have some 850 generic tests in gcc/testsuite that use __SIZE_TYPE__
> > without including stddef.h. They just rely on the preprocessor to expand this
> > using the builtin macro definition.  
> 
> I did say it is a dirty hack, right?
> 
> You could call lang_hooks.types.register_builtin_type defining the type
> __SIZE_TYPE__ as the int20 partial int type, and then define SIZE_TYPE as
> just __SIZE_TYPE__.  That is the same effectively, just not using the
> header file.
> 
> So something like
> 
>   tree type_node = builtin_type_for_size (20, 1);
>   lang_hooks.types.register_builtin_type (type_node, "__SIZE_TYPE__");
> 
> or maybe
> 
>   tree type_node = builtin_type_for_size (POINTER_SIZE, 1);
>   lang_hooks.types.register_builtin_type (type_node, "__SIZE_TYPE__");
> 
> in msp430.c, and
> 
>   #define SIZE_TYPE "__SIZE_TYPE__"
> 
> in msp430.h?
> 
> 
> Segher

Thanks for the pointers, they've put me on the right track I think.

It doesn't look like we can create the new type in the msp430 backend - the
SIZE_TYPE macro is examined quite early and only a couple of basic backend
initialization functions are called before SIZE_TYPE is needed in
c_common_nodes_and_builtins(). TARGET_INIT_BUILTINS seemed most appropriate,
but by then it's too late to create the type and use it in msp430.h.

Instead I fixed it by creating a new type "__int20__" in the middle-end,
which can then be used for SIZE_TYPE in msp430.h.
__int20__ is not really a proper type - it shares it's "RID" values with 
__int20, but it means we can gate the ISO C pedwarn so it only is emitted for
__int20 and not __int20__. It's ok for __int20 and __int20__ to have identical
behavior, aside from the pedwarn, which is why sharing the RIDs should be ok.

I think this solution fits ok with the existing behavior related to "pedantic"
warnings, since alternate keywords beginning and ending
with "__" are considered GNU Extensions and don't pedwarn. I guess "__int20"
isn't technically a "keyword" in the same sense as "asm" for example. But
its a "reserved word" and this fix fits this pattern of surrounding with
double-underscores.

Any thoughts on this approach in my attached (rough) patch? So far I regtested
it fine with the GCC C testsuite for msp430-elf.
Also need to do the same for PTRDIFF_TYPE and make that use __int20__.

I initially implemented this by giving the __intN__ types their own new set of
RIDs so they are completely distinct from __intN, but it requires quite a lot
of duplicated code whenever RID_INT_N_* or RID_{FIRST,LAST}_INT_N are used,
just to handle the new RIDs. This patch is at least quite concise and gets the
job done.
Of course, if the __intN__ types really should have their own unique RIDs
then I can do it that way instead.

Thanks,
Jozef