On Mon, Mar 27, 2017 at 11:48 PM, Martin Jambor <mjam...@suse.cz> wrote:
> Hi,
>
> On Mon, Mar 27, 2017 at 12:15:10PM -0600, Jeff Law wrote:
>> On 03/27/2017 05:40 AM, Martin Liška wrote:
>> > Hello.
>> >
>> > There's alternative approach suggested by Martin Jambor.
>> > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests
>> > and
>> > s390x cross compiler does not ICE.
>> >
>> > Martin
>> >
>> > 2017-03-23 Martin Liska <mli...@suse.cz>
>> >
>> > PR ipa/80104
>> > * cgraphunit.c (cgraph_node::expand_thunk): Mark argument of a
>> > thunk call as DECL_GIMPLE_REG_P when vector or complex type.
>> Can you fix the documentation for DECL_GIMPLE_REG_P to indiate that it can
>> be set on parameters.
>>
>> In gimplify_function_tree we have this:
>>
>> for (parm = DECL_ARGUMENTS (fndecl); parm ; parm = DECL_CHAIN (parm))
>> {
>> /* Preliminarily mark non-addressed complex variables as eligible
>> for promotion to gimple registers. We'll transform their uses
>> as we find them. */
>> if ((TREE_CODE (TREE_TYPE (parm)) == COMPLEX_TYPE
>> || TREE_CODE (TREE_TYPE (parm)) == VECTOR_TYPE)
>> && !TREE_THIS_VOLATILE (parm)
>> && !needs_to_live_in_memory (parm))
>> DECL_GIMPLE_REG_P (parm) = 1;
>> }
>>
>> Aren't you essentially doing the same thing for thunks?
>
> Yes.
>
>> Does it make sense
>> to pull that into a little function and just call it from both places?
>
> Possibly...
>
>>
>> If not, do we need to add the !TREE_THIS_VOLATILE and
>> !needs_to_live_in_memory checks to the thunks version?
>
> ...although if any of these checks fail, the bug will re-surface.
>
> I do not really know what a volatile parameter means, let alone a
> volatile parameter of a hunk. Unless, I am mistaken, hunk parameters
> are never made TREE_ADDRESSABLE, so needs_to_live_in_memory can be
> omitted.
Yeah, I think the thunk case is quite constrained so the revised patch
is ok with me.
The volatile check is somewhat superfluous as even DECL_GIMPLE_REG_P volatiles
are not written into-SSA (is_gimple_reg () still returns false). Likewise for
needs_to_live_in_memory.
Thanks,
Richard.
>>
>> Generally OK, just want to work through these couple niggling details.
>>
>
> Thanks for looking at this,
>
> Martin
