On 10/26/18 12:59 AM, Michael Ploujnikov wrote:
> I've taken the advice from a discussion on IRC and re-wrote the patch
> with more uniform function names and using overloading.
>
> I think this function accomplished the following goals:
> - remove clone numbering where it's not needed:
> final.c:final_scan_insn_1 and
> symtab.c:simd_symtab_node::noninterposable_alias.
> - name and document the clone naming API such that future users won't
> accidentally use the numbering when it's not necessary; if you need
> numbering then you need to explicitly ask for it with the right
> function
> - provide a new function that allows users to specify a clone number
> explicitly as an argument
Hello.
Thanks for reworking that.
>
> My thoughts for future improvements:
> - It's a bit unfortunate that lto-partition.c:privatize_symbol_name_1
> has to break the decl abstraction and pass in a string that it
> created into what I would consider the implementation-detail
> function. The best way I can think of to make it uniform with the
> rest of the users is to have it create a new empty decl with
> DECL_ASSEMBLER_NAME set to the new string
That's not nice to create artificial declaration. Having string variant
is fine for me.
> - It's unfortunate that I have to duplicate the separator
> concatenation in the numberless clone_function_name, but I think it
> has to be like that unless ASM_FORMAT_PRIVATE_NAME making the
> number optional.
>
That's also fine for me. I'm attaching small nits that I found.
And please reformat following chunk in ChangeLog entry:
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only. (clone_function_name):
Renamed to clone_function_name_numbered to be explicit about
numbering. (clone_function_name): New two-argument function
that does not number its output. (clone_function_name): New
three-argument function that takes a number to append to its
output.
into:
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only.
(clone_function_name): Renamed to clone_function_name_numbered
to be explicit about...
I'm adding Honza to CC, hope he can review it quickly.
Thanks,
Martin
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index c896a5f60cb..9cba4c2c3a9 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -521,6 +521,7 @@ static GTY(()) unsigned int clone_fn_id_num;
then the two argument clone_function_name should be used instead.
Should not be called directly except for by
lto-partition.c:privatize_symbol_name_1. */
+
tree
clone_function_name_numbered (const char *name, const char *suffix)
{
@@ -532,6 +533,7 @@ clone_function_name_numbered (const char *name, const char *suffix)
assembler name) unspecified number. If clone numbering is not
needed then the two argument clone_function_name should be used
instead. */
+
tree
clone_function_name_numbered (tree decl, const char *suffix)
{
@@ -542,8 +544,9 @@ clone_function_name_numbered (tree decl, const char *suffix)
/* Return a new assembler name for a clone of decl named NAME. Apart
from the string SUFFIX, the new name will end with the specified
- number. If clone numbering is not needed then the two argument
+ NUMBER. If clone numbering is not needed then the two argument
clone_function_name should be used instead. */
+
tree
clone_function_name (const char *name, const char *suffix,
unsigned long number)
@@ -559,9 +562,9 @@ clone_function_name (const char *name, const char *suffix,
return get_identifier (tmp_name);
}
-
/* Return a new assembler name ending with the string SUFFIX for a
clone of DECL. */
+
tree
clone_function_name (tree decl, const char *suffix)
{
@@ -581,7 +584,7 @@ clone_function_name (tree decl, const char *suffix)
IDENTIFIER_POINTER (identifier),
separator,
suffix,
- (char*)0));
+ NULL));
return get_identifier (result);
}
