On Sun, Nov 6, 2011 at 12:16 AM, Aldy Hernandez <al...@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
>
