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] Fix calls.c for a _complex type (PR ipa/80104).


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


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