This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] 19/n: trans-mem: compiler tree/gimple stuff
On Sun, Nov 6, 2011 at 12:16 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> [rth, see below]
>
>>> Index: gcc/attribs.c
>>> ===================================================================
>>> --- gcc/attribs.c ? ? ? (.../trunk) ? ? (revision 180744)
>>> +++ gcc/attribs.c ? ? ? (.../branches/transactional-memory) ? ? (revision
>>> 180773)
>>> @@ -166,7 +166,8 @@ init_attributes (void)
>>> ? ? ? ? ?gcc_assert (strcmp (attribute_tables[i][j].name,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?attribute_tables[i][k].name));
>>> ? ? }
>>> - ?/* Check that no name occurs in more than one table. ?*/
>>> + ?/* Check that no name occurs in more than one table. ?Names that
>>> + ? ? begin with '*' are exempt, and may be overridden. ?*/
>>> ? for (i = 0; i< ?ARRAY_SIZE (attribute_tables); i++)
>>> ? ? {
>>> ? ? ? size_t j, k, l;
>>> @@ -174,8 +175,9 @@ init_attributes (void)
>>> ? ? ? for (j = i + 1; j< ?ARRAY_SIZE (attribute_tables); j++)
>>> ? ? ? ?for (k = 0; attribute_tables[i][k].name != NULL; k++)
>>> ? ? ? ? ?for (l = 0; attribute_tables[j][l].name != NULL; l++)
>>> - ? ? ? ? ? gcc_assert (strcmp (attribute_tables[i][k].name,
>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? attribute_tables[j][l].name));
>>> + ? ? ? ? ? gcc_assert (attribute_tables[i][k].name[0] == '*'
>>> + ? ? ? ? ? ? ? ? ? ? ? || strcmp (attribute_tables[i][k].name,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?attribute_tables[j][l].name));
>>> ? ? }
>>> ?#endif
>>>
>>> @@ -207,7 +209,7 @@ register_attribute (const struct attribu
>>> ? slot = htab_find_slot_with_hash (attribute_hash,&str,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? substring_hash (str.str, str.length),
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? INSERT);
>>> - ?gcc_assert (!*slot);
>>> + ?gcc_assert (!*slot || attr->name[0] == '*');
>>> ? *slot = (void *) CONST_CAST (struct attribute_spec *, attr);
>>> ?}
>>
>> The above changes seem to belong to a different changeset and look
>> strange. ?Why would attributes ever appear in two different tables?
>
> I couldn't find a corresponding gcc-patches message for this patch, but I
> was able to hunt down full the patch, which I am attaching for discussion.
>
> This seems to be a change required for allowing '*' to override builtins, so
> it is indeed part of the branch. ?Perhaps with the full context it is easier
> to review.
Ah, indeed ...
> I will defer to rth to answer any questions on the original motivation.
>
> Richi, do you have any particular issue with the attribs.c change? ?Does
> this context resolve any questions you may have had?
... no, it just looked weird without seeing a use. Now, target specific
attributes on a non-target specific builtin are of course weird. Which
explains the patch, sort-of. Still feels like a hack, but I can't think
of anything better, other than a target hook that we'd call for
all middle-end builtins we generate and which would allow target specific
modifications. No idea if that would be better. I'll defer to rth for this.
Richard.
> Aldy
>