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 >