Hi Jozef,
On Tue, Jun 11, 2019 at 09:44:30PM +0100, Jozef Lawrynowicz wrote:
> 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__.
Ooh I like this :-) Fits in well everywhere, and it's nicely generic, too.
> It's ok for __int20 and __int20__ to have identical
> behavior, aside from the pedwarn, which is why sharing the RIDs should be ok.
Many other keywords do the same, see asm/__asm/__asm__ for example.
> 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__.
> --- a/gcc/lto/lto-lang.c
> +++ b/gcc/lto/lto-lang.c
> @@ -1260,9 +1260,9 @@ lto_build_c_type_nodes (void)
> if (int_n_enabled_p[i])
> {
> char name[50];
> - sprintf (name, "__int%d unsigned", int_n_data[i].bitsize);
> + sprintf (name, "int%d", int_n_data[i].bitsize);
>
> - if (strcmp (name, SIZE_TYPE) == 0)
> + if (strstr (SIZE_TYPE, name) != NULL)
> {
> intmax_type_node = int_n_trees[i].signed_type;
> uintmax_type_node = int_n_trees[i].unsigned_type;
I don't think that is correct, strstr allows too much? If you want to
allow some variants, you should test for all those variants exactly?
It looks great otherwise :-)
Segher
