[PATCH] Fix calls.c for a _complex type (PR ipa/80104).

[Jeff Law]

On 03/28/2017 02:09 AM, Richard Biener wrote:
> On Mon, Mar 27, 2017 at 11:48 PM, Martin Jambor <mjambor@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  <mliska@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.
Works for me -- I raised those potential issues because of the 
inconsistency with the existing code for normal functions.  I did not 
dig into why we have those additional checks for normal functions.

So, no objections here.

Jeff