Re: [PATCH] New attribute to create target clones

[Jeff Law <law@redhat.com>]

On 10/29/2015 12:13 PM, Evgeny Stupachenko wrote:
> On Thu, Oct 29, 2015 at 8:02 PM, Jan Hubicka<hubicka@ucw.cz>  wrote:
>>> >>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.
>> >
>> >I am not against more informative names, but why you don't pass the info here:
>> >
>> >+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->force_output = true;
>> >+    }
>> >+  else
>> >+    {
>> >+      tree new_decl = copy_node (node->decl);
>> >+      new_node = cgraph_node::get_create (new_decl);
>> >+    }
>> >+  return new_node;
>> >+}
>> >
>> >passing "arch_slm" instead of target_clone will get you the name you want
>> >(plus the extra index that may be needed anyway to disambiguate).
>> >
>> >Note that in general those .suffixes should be machine parseable, so cp-demangle.c
>> >can expand them correctly.  We may want to have some consistent grammar for them here
>> >and update cp-demangle.c to output nice info like "target clone for..."
> Ok. I've modified the patch correspondingly.
You'll need updated ChangeLog entries.  Don't forget to drop the omp-low 
spurious whitespace change.

You should also fix the formatting nits Jan pointed out.

With those changes, this patch is OK for the trunk.  I'll run the header 
file reordering & cleanup tool after the patch is committed to the trunk.

jeff