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.
Ready for trunk?
Thanks,
Martin
gcc/ChangeLog:
2014-11-07 Martin Liska <mli...@suse.cz>
* cgraphunit.c (cgraph_node::create_wrapper):
TREE_ADDRESSABLE is set to false for a newly created thunk.
gcc/testsuite/ChangeLog:
2014-11-07 Martin Liska <mli...@suse.cz>
* g++.dg/ipa/pr63580.C: New test.
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 6f61f5c..89c96dc 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2342,6 +2342,14 @@ cgraph_node::create_wrapper (cgraph_node *target)
cgraph_edge *e = create_edge (target, NULL, 0, CGRAPH_FREQ_BASE);
+ tree arguments = DECL_ARGUMENTS (decl);
+
+ while (arguments)
+ {
+ TREE_ADDRESSABLE (arguments) = false;
+ arguments = TREE_CHAIN (arguments);
+ }
+
expand_thunk (false, true);
e->call_stmt_cannot_inline_p = true;
diff --git a/gcc/testsuite/g++.dg/ipa/pr63580.C b/gcc/testsuite/g++.dg/ipa/pr63580.C
new file mode 100644
index 0000000..904195a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr63580.C
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf" } */
+
+struct A
+{
+};
+template <class L, class R> A operator%(L, R);
+template <class A0, class A1, class A2, class A3>
+void make_tuple (A0 &, A1, A2, A3);
+A
+bar (int p1, char p2, int p3, double p4)
+{
+ A a;
+ make_tuple (p1, p2, p3, p4);
+ return "int; char; string; double; " % a;
+}
+A
+foo (int p1, char p2, int p3, double p4)
+{
+ A b;
+ make_tuple (p1, p2, p3, p4);
+ return "int; char; string; double; " % b;
+}
+
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
