Message ID | 545A2FB4.8060509@arm.com |
---|---|
State | New |
Headers | show |
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. jeff
> 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
diff --git a/gcc/cgraph.c b/gcc/cgraph.c index d430bc5..7ac0b2a 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1668,6 +1668,7 @@ release_function_body (tree decl) void cgraph_node::release_body (bool keep_arguments) { + tree arg_p; ipa_transforms_to_apply.release (); if (!used_as_abstract_origin && symtab->state != PARSING) { @@ -1676,6 +1677,10 @@ cgraph_node::release_body (bool keep_arguments) if (!keep_arguments) DECL_ARGUMENTS (decl) = NULL; } + + for (arg_p = DECL_ARGUMENTS (decl); arg_p; arg_p = DECL_CHAIN (arg_p)) + TREE_ADDRESSABLE (arg_p) = 0; + /* If the node is abstract and needed, then do not clear DECL_INITIAL of its associated function function declaration because it's needed to emit debug info later. */