Re: [PATCH] PR 63721 IPA ICF cause atomic-comp-swap-release-acquire.c ICE

[Jiong Wang]

On 07/11/14 12:53, Martin Liška wrote:

On 11/07/2014 10:52 AM, Jan Hubicka wrote:

On 11/05/14 07:09, Jiong Wang wrote:

the same ICE will happen on x86-64, if compile with -O2 -fPIC.

the reason is for the following two functions, they are identical, so
IPA-ICF
pass try to transform the second function to call the first one directly.

int
atomic_compare_exchange_STRONG_RELEASE_ACQUIRE (int a, int b)
{
   return __atomic_compare_exchange (&v, &a, &b,
                     STRONG, __ATOMIC_RELEASE,
                     __ATOMIC_ACQUIRE);
}

int
atomic_compare_exchange_n_STRONG_RELEASE_ACQUIRE (int a, int b)
{
   return __atomic_compare_exchange_n (&v, &a, b,
                       STRONG, __ATOMIC_RELEASE,
                       __ATOMIC_ACQUIRE);
}

while during this transformation, looks like there are something wrong
with the
function argument handling. take "a" for example, because later there
are "&a",
so it's marked as addressable. while after transformation, if we turn the
second function into

int atomic_compare_exchange_n_STRONG_RELEASE_ACQUIRE (int a, int b)
{
   return atomic_compare_exchange_STRONG_RELEASE_ACQUIRE (a, b)
}

then argument a is no longer addressable.

so, in cgraph_node::release_body, when making the wrapper, except
clearing the
function body, we should also clear the addressable flag for function args
because they are decided by the function body which is cleared.

bootstrap ok on x86-64 and no regression.
bootstrap ok on aarch64 juno.
ICE gone away on arm & x86-64

ok for trunk?

gcc/

   PR  tree-optimization/63721
   * cgraph.c (cgraph_node::release_body): Clear addressable flag for
function args.

While I understand the need to clear the addressable flag, I think
release_body probably isn't the best place to do this.  Seems to me
that ought to happen when we emit the thunk or otherwise transform
the body into something that doesn't take the address of those
parameters.

Yep, I would just move it into expand_thunk - the TREE_ADDRESSABLE bits are not 
really
well defined before we build the gimple body.

Honza

jeff

Hello.

I think the bug is a duplicate of PR63580 and there's working patch that can 
bootstrap on x86_64-linux and no regression has been seen.

thanks.

looks good to me, although I think expand_thunk is a better place to fix as 
there is a loop on arguments already,

      for (arg = a; arg; arg = DECL_CHAIN (arg))
        nargs++;

Ready for trunk?
Thanks,
Martin