This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] New attribute to create target clones


On 10/08/2015 03:36 PM, Jan Hubicka wrote:

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.

OK, I assume that every multiversioned function will end up in something
like this:

foo_ver1() target(...)
{
}
foo_ver2() target(...)
{
}
foo()
{
   dispatch either to foo_ver1() or foo_ver2()
}
Yes.


I wonder why foo_ver1/foo_ver2 needs ever become public?  If there is only way
to call them via dispatcher, then the code setting TREE_PUBLIC seems wrong.  If
there is direct way to call them, then inlinng is possible.
I can't offhand think of a good reason why they should be public. In fact, it would defeat the purpose of this feature to begin with if those symbols were directly reachable. So, yes, setting TREE_PUBLIC seems wrong.



Of course it also depends what you inline into function. You can have

bar() target(-mavx) {fancy avx code}
foobar() { ...... if (avx) bar();}
foo() ctarget(-mavx,-mno-avx) {....foobar();....}

Now if you compile with -mavx and because ctarget takes effect only after inlining,
at inlining time the target attributes will match and we can edn up inline bar->foobar->foo.
After that we multiversion foo and drop AVX flag we will likely get ICE at expansion
time.
But isn't that avoided by fixing up the call graph so that all calls to the affected function are going through the dispatcher? Or is that happening too late?




Since we're going through a dispatcher I don't think we're allowed
to change the ABI across the clones.

If the dispatcher was something like

switch(value)
{
   case 1: foo_ver1(param); break;
   case 2: foo_ver2(param); break;
}
then it would be possible to change ABI if we can prove that param=0 in ipa-cp.
If the dispatcher uses indirect calls, then we probably want to consider
foo_ver* as having address taken and thus not local.
Yea, I guess that could work. For some reason I kept thinking about indirect calls, but that shouldn't be the case here.

That argues further that setting TREE_PUBLIC on the target specific implementations is wrong.


Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]