On 10/08/2015 01:23 PM, Jan Hubicka wrote:
Sorry this has taken so long to come back to... As I mentioned a
couple months ago, I'd hoped Jan would chime in on the IPA/symtab
requirements. But that didn't happen.
Sorry for that. I had bit too many real life things this summer
and I am still trying to catch up.
It happens to us all :-) No worries.
Ilya's code seems different from what this patch does. Ilya simply needs
multiple declarations for one physical assembler name (this is not an alias).
This is not currently supported by symtab (support for that was removed long
time ago as part of the one decl project) and I have some perliminary patches
to push out, but since they add basic sanity checking that the different
declarations of the same thing looks compatible I get too many positives I need
to walk through. Those seems real bugs in glibc (which uses duplicated decls
for checking) and the pointer bounds code.
Right. I was mostly concerned because I goof'd letting those bits from
Ilya in and didn't want to repeat the mistake again. It was pretty
clear once I found the thread between the tree of us that Evgeny's
patches were doing something rather different and didn't violate the
assumptions of the symtab code.
Yes, here you have different names for different variants of the function
body. Basically this pass takes ctarget attribute and creates bunch of verisons
of the functions and assigns them the proper target attributes, right?
Right. Given a single function in the source tree with the new
attribute, we'll create clones and compile each clone with a different
set of options. It's got a lot of similarities to the multi-versioning
code. The key difference is with multi-versioning, you actually have a
different source level implementation for each target while Evgeny's
stuff has a single source implementation.
One thing I am confused about is why this does not happen early?
What happens to inlines of functions with specific taret requirements? I.e.
if I compile with AVX enabled have function body with AVX code but then, at
late compilation, force a clone with -mno-avx?
I would expect cloning to happen early, perhaps even before early
optimizations...
Switching random target flags mid optimization queue seems dangerous.
These shouldn't be inlined to the best of my knowledge. We go back and
direct all callers to a dispatcher. Inlining them would be a mistake
since the goal here is to specialize the clones around target
capabilities. Thus if something got inlined, then we lose that ability.
As for the patch itself:
+ if (node->definition)
+ {
+ if (!node->has_gimple_body_p ())
+ return false;
+ node->get_body ();
+
+ /* Replacing initial function with clone only for 1st ctarget. */
+ 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;
+ TREE_PUBLIC (new_node->decl) = TREE_PUBLIC (node->decl);
+ }
Since you test it has gimple body, then you don't need to worry about
alias/thunk/weakrefs/implicit_alias properties. Those will be never set.
How does the dispatcher look like? Can the function be considered local
in a sense that one can change calling conventions of one clone but not another?
Easiest to think of them as having the same abilities as multi-versioning.
Since we're going through a dispatcher I don't think we're allowed to
change the ABI across the clones.
Also when you are copying a function, you probably want to copy the associated
thunks and version them, too?
Dunno on that.
Jeff
