On Mon, Oct 26, 2015 at 6:50 PM, Jeff Law <l...@redhat.com> wrote: > On 10/12/2015 05:35 PM, Evgeny Stupachenko wrote: >> >> Hi All, >> >> Here is a new version of patch (attached). >> Bootstrap and make check are in progress (all new tests passed). >> >> New test case g++.dg/ext/mvc4.C fails with ICE, when options lower >> than "-mavx" are passed. >> However it has the same behavior if "target_clones" attribute is >> replaced by 2 corresponding "target" attributes. >> I've filed PR67946 on this: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67946 >> >> Thanks, >> Evgeny >> >> ChangeLog: >> >> 2015-10-13 Evgeny Stupachenko<evstu...@gmail.com> >> gcc/ >> * Makefile.in (OBJS): Add multiple_target.o. >> * attrib.c (make_attribute): Moved from config/i386/i386.c >> * config/i386/i386.c (make_attribute): Deleted. >> * multiple_target.c (make_attribute): New. >> (create_dispatcher_calls): Ditto. >> (get_attr_len): Ditto. >> (get_attr_str): Ditto. >> (is_valid_asm_symbol): Ditto. >> (create_new_asm_name): Ditto. >> (create_target_clone): Ditto. >> (expand_target_clones): Ditto. >> (ipa_target_clone): Ditto. >> (ipa_dispatcher_calls): Ditto. >> * passes.def (pass_target_clone): Two new ipa passes. >> * tree-pass.h (make_pass_target_clone): Ditto. >> >> gcc/c-family >> * c-common.c (handle_target_clones_attribute): New. >> * (c_common_attribute_table): Add handle_target_clones_attribute. >> * (handle_always_inline_attribute): Add check on target_clones >> attribute. >> * (handle_target_attribute): Ditto. >> >> gcc/testsuite >> * gcc.dg/mvc1.c: New test for multiple targets cloning. >> * gcc.dg/mvc2.c: Ditto. >> * gcc.dg/mvc3.c: Ditto. >> * gcc.dg/mvc4.c: Ditto. >> * gcc.dg/mvc5.c: Ditto. >> * gcc.dg/mvc6.c: Ditto. >> * gcc.dg/mvc7.c: Ditto. >> * g++.dg/ext/mvc1.C: Ditto. >> * g++.dg/ext/mvc2.C: Ditto. >> * g++.dg/ext/mvc3.C: Ditto. >> * g++.dg/ext/mvc4.C: Ditto. >> >> gcc/doc >> * doc/extend.texi (target_clones): New attribute description. >> > >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >> index 23e6a76..f9d28d1 100644 >> --- a/gcc/doc/extend.texi >> +++ b/gcc/doc/extend.texi >> @@ -3066,6 +3066,19 @@ This function attribute make a stack protection of >> the function if >> flags @option{fstack-protector} or @option{fstack-protector-strong} >> or @option{fstack-protector-explicit} are set. >> >> +@item target_clones (@var{options}) >> +@cindex @code{target_clones} function attribute >> +The @code{target_clones} attribute is used to specify that a function is >> to >> +be cloned into multiple versions compiled with different target options >> +than specified on the command line. The supported options and >> restrictions >> +are the same as for @code{target}. > > "as for @code{target}" -> "as for the @code{target} attribute." > > I think that makes it a tiny bit clearer. > > > > >> + >> +/* Creates target clone of NODE. */ >> + >> +static cgraph_node * >> +create_target_clone (cgraph_node *node, bool definition) >> +{ >> + cgraph_node *new_node; >> + if (definition) >> + { >> + new_node = node->create_version_clone_with_body (vNULL, NULL, >> + NULL, false, >> + NULL, NULL, >> + "target_clone"); >> + new_node->externally_visible = node->externally_visible; >> + new_node->address_taken = node->address_taken; >> + new_node->thunk = node->thunk; >> + new_node->alias = node->alias; >> + new_node->weakref = node->weakref; >> + new_node->cpp_implicit_alias = node->cpp_implicit_alias; >> + new_node->local.local = node->local.local; > > So do we need to explicitly clear TREE_PUBLIC here? It also seems like > copying externally_visible, address_taken and possibly some of those other > fields is wrong. The clone is going to be local to the CU, it doesn't > inherit those properties from the original -- only the dispatcher needs to > inherit those properties, right? Right. That was just the way to keep the node, that I didn't clean up. Replaced with "new_node->force_output = true;"
>
>> +
>> +
>> + for (i = 0; i < attrnum; i++)
>> + {
>> + char *attr = attrs[i];
>> + cgraph_node *new_node = create_target_clone (node, defenition);
>> + char *new_asm_name =
>> + XNEWVEC (char, strlen (old_asm_name) + strlen (attr) + 2);
>> + create_new_asm_name (old_asm_name, attr, new_asm_name);
>
> I thought after discussions with Jan that this wasn't going to be necessary
> as cloning should create a suitable name for us?
Yes. This is not necessary. However that way we'll have the following
code in dispatcher:
cmpl $6, __cpu_model+4(%rip)
sete %al
movzbl %al, %eax
testl %eax, %eax
jle .L16
movl $foo.target_clone.1, %eax
I think it is very hard to read and debug such...
While now we have:
cmpl $6, __cpu_model+4(%rip)
sete %al
movzbl %al, %eax
testl %eax, %eax
jle .L16
movl $foo.arch_slm, %eax
and it is clear that we are jumping to SLM code here.
I'd like to keep target in names.
Thanks,
Evgeny
>
>
> Jeff
target_clones4.patch
Description: Binary data
