Re: [google] Patch to support calling multi-versioned functions via new GCC builtin. (issue4440078)

[Xinliang David Li <davidxl at google dot com>]

On Mon, May 2, 2011 at 2:33 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Mon, May 2, 2011 at 6:41 PM, Xinliang David Li <davidxl@google.com> wrote:
>> On Mon, May 2, 2011 at 2:11 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Fri, Apr 29, 2011 at 6:23 PM, Xinliang David Li <davidxl@google.com> wrote:
>>>> Here is the background for this feature:
>>>>
>>>> 1) People relies on function multi-version to explore hw features and
>>>> squeeze performance, but there is no standard ways of doing so, either
>>>> a) using indirect function calls with function pointers set at program
>>>> initialization; b) using manual dispatch at each callsite; b) using
>>>> features like IFUNC. ?The dispatch mechanism needs to be promoted to
>>>> the language level and becomes the first class citizen;
>>>
>>> You are not doing that, you are inventing a new (crude) GCC extension.
>>
>> To capture the high level semantics and prevent user from lowering the
>> dispatch calls into forms compiler can not recognize, language
>> extension is the way to go.
>
> I don't think so. ?With your patch only two passes understand the new
> high-level form, the rest of the gimple passes are just confused.

There is no need for other passes to understand it -- just treat it as
opaque calls. This is goodness otherwise other passes need to be
modified. This is true (only some passes understand it) for things
like __builtin_expect.


>>
>> 1) the desired optimization may not happen subject to many compiler
>> heuristic changes;
>> 2) it has other side effects such as wrong estimation of function size
>> which may impact inlining
>
> May, may ... so you say all this can't happen under any circumstance
> with your special code and passes?

No that is not my argument. What I tried to say is it will be harder
to achieve without high level semantics -- it requires more
handshaking between compiler passes.

> Which nobody will see benefit
> from unless they rewrite their code?

The target users for the builtin include compiler itself -- it can
synthesize dispatch calls.

>?Well, I say if we can improve
> _some_ of the existing usages that's better than never doing wrong
> on a new language extension.

This is independent.

>?One that I'm not convinced is the way
> to go (you didn't address at all the inability to use float arguments
> and the ABI issues with using variadic arguments - after all you
> did a poor-mans language extension by using GCC builtins instead
> of inventing a true one).

This is an independent issue that either needs to be addressed or
marked as limitation. The key of the debate is whether source/IR
annotation using construct with high level semantics helps optimizer.
In fact this is common. Would it make any difference (in terms of
acceptance) if the builtin is only used internally by the compiler and
not exposed to the user?

>
>> 3) it limits the lowering into one form which may not be ideal ?--
>> with builtin_dispatch, after hoisting optimization, the lowering can
>> use more efficient IFUNC scheme, for instance.
>
> I see no reason why we cannot transform a switch-indirect-call
> pattern into an IFUNC call.
>

It is possible -- but it is like asking user to lower the dispatch and
tell compiler to raise it again ..

>>> My point is that such optimization is completely independent of
>>> that dispatch thing. ?The above could as well be a selection to
>>> use different input arrays, one causing alias analysis issues
>>> and one not.
>>>
>>> Thus, a __builtin_dispatch centric optimization pass is the wrong
>>> way to go.
>>
>> I agree that many things can implemented in different ways, but a high
>> level standard builtin_dispatch mechanism doing hoisting
>> interprocedcurally is cleaner and simpler and targeted for function
>> multi-versioning -- it does not depend on/rely on later phase's
>> heuristic tunings to make the right things to happen. Function MV
>> deserves this level of treatment as it will become more and more
>> important for some users (e.g., Google).
>
> But inventing a new language extension to benefit from whatever
> improvements we implement isn't the obviously best choice.

It is not for any improvement. I mentioned the potential for function
MV and want to have a compiler infrastructure to deal with it.


>
>>> Now, with FDO I'd expect the foo is inlined into bar (because foo
>>> is deemed hot),
>>
>> That is a myth -- the truth is that there are other heuristics which
>> can prevent this from happening.
>
> Huh, sure. ?That doesn't make my expectation a myth.
>
>>> then you only need to deal with loop unswitching,
>>> which should be easy to drive from FDO.
>>
>> Same here -- the loop body may not be well formed/recognized. The loop
>> nests may not be perfectly nested, or other unswitching heuristics may
>> block it from happening. ?This is the common problem form many other
>> things that get lowered too early. It is cleaner to make the high
>> level transformation first in IPA, and let unswitching dealing with
>> intra-procedural optimization.
>
> If it's not well-formed inlining the call does not make it well-formed and
> thus it won't be optimized well anyway. ?Btw, for the usual cases I
> have seen inlining isn't a transform that is worthwhile - transforming
> to IFUNC would have been.

I am not sure I understand the comment here. The proposed approach can
do interprocedural hoisting of the dispatch and this done pretty early
in the pipeline so that hot functions can be optimized as much as
possible. Lowering it early in the hot functions rely later phases to
deal with it has the following limitations:

0) lowered code can get in the way of effective optimization
1) the lowered code can be transformed such that it can not be
unswitched or can not be raised properly
2) all functions in the related call chain to be inlined for the
unswitching to happen interprocedurally
3) relies on unswitching heuristic to kick in

thanks,

David

>
> I'm not convinced at all.
>
> Richard.
>