* [PATCH, GCC/LTO] Fix PR69866: LTO with def for weak alias in regular object file
@ 2017-03-02 14:52 Thomas Preudhomme
2017-03-09 9:43 ` Thomas Preudhomme
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2017-03-02 14:52 UTC (permalink / raw)
To: Richard Biener, gcc-patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]
Hi,
This patch fixes an assert failure when linking one LTOed object file
having a weak alias with a regular object file containing a strong
definition for that same symbol. The patch is twofold:
+ do not add an alias to a partition if it is external
+ do not declare (.globl) an alias if it is external
ChangeLog entries are as follow:
*** gcc/lto/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
aliases to partition.
*** gcc/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
declare external aliases.
*** gcc/testsuite/ChangeLog ***
2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* gcc.dg/lto/pr69866_0.c: New test.
* gcc.dg/lto/pr69866_1.c: Likewise.
Testing: Testsuite shows no regression when targeting Cortex-M3 with an
arm-none-eabi GCC cross-compiler, neither does it show any regression with
native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
Is this ok for stage4?
Best regards,
Thomas
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO] Fix PR69866: LTO with def for weak alias in regular object file
2017-03-02 14:52 [PATCH, GCC/LTO] Fix PR69866: LTO with def for weak alias in regular object file Thomas Preudhomme
@ 2017-03-09 9:43 ` Thomas Preudhomme
2017-03-16 14:05 ` [PATCH, GCC/LTO, ping2] " Thomas Preudhomme
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2017-03-09 9:43 UTC (permalink / raw)
To: Richard Biener, gcc-patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]
Ping?
Best regards,
Thomas
On 02/03/17 14:51, Thomas Preudhomme wrote:
> Hi,
>
> This patch fixes an assert failure when linking one LTOed object file
> having a weak alias with a regular object file containing a strong
> definition for that same symbol. The patch is twofold:
>
> + do not add an alias to a partition if it is external
> + do not declare (.globl) an alias if it is external
>
> ChangeLog entries are as follow:
>
> *** gcc/lto/ChangeLog ***
>
> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> PR lto/69866
> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
> aliases to partition.
>
> *** gcc/ChangeLog ***
>
> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> PR lto/69866
> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
> declare external aliases.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> PR lto/69866
> * gcc.dg/lto/pr69866_0.c: New test.
> * gcc.dg/lto/pr69866_1.c: Likewise.
>
>
> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
> arm-none-eabi GCC cross-compiler, neither does it show any regression with
> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>
> Is this ok for stage4?
>
> Best regards,
>
> Thomas
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping2] Fix PR69866: LTO with def for weak alias in regular object file
2017-03-09 9:43 ` Thomas Preudhomme
@ 2017-03-16 14:05 ` Thomas Preudhomme
2017-03-23 16:08 ` [PATCH, GCC/LTO, stage4, ping3] " Thomas Preudhomme
2017-03-31 15:25 ` [PATCH, GCC/LTO, ping2] " Jeff Law
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2017-03-16 14:05 UTC (permalink / raw)
To: Richard Biener, gcc-patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]
Ping?
Is this ok for stage4?
Best regards,
Thomas
On 09/03/17 09:43, Thomas Preudhomme wrote:
> Ping?
>
> Best regards,
>
> Thomas
>
> On 02/03/17 14:51, Thomas Preudhomme wrote:
>> Hi,
>>
>> This patch fixes an assert failure when linking one LTOed object file
>> having a weak alias with a regular object file containing a strong
>> definition for that same symbol. The patch is twofold:
>>
>> + do not add an alias to a partition if it is external
>> + do not declare (.globl) an alias if it is external
>>
>> ChangeLog entries are as follow:
>>
>> *** gcc/lto/ChangeLog ***
>>
>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>
>> PR lto/69866
>> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
>> aliases to partition.
>>
>> *** gcc/ChangeLog ***
>>
>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>
>> PR lto/69866
>> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
>> declare external aliases.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>
>> PR lto/69866
>> * gcc.dg/lto/pr69866_0.c: New test.
>> * gcc.dg/lto/pr69866_1.c: Likewise.
>>
>>
>> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
>> arm-none-eabi GCC cross-compiler, neither does it show any regression with
>> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>>
>> Is this ok for stage4?
>>
>> Best regards,
>>
>> Thomas
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, stage4, ping3] Fix PR69866: LTO with def for weak alias in regular object file
2017-03-16 14:05 ` [PATCH, GCC/LTO, ping2] " Thomas Preudhomme
@ 2017-03-23 16:08 ` Thomas Preudhomme
2017-03-31 15:25 ` [PATCH, GCC/LTO, ping2] " Jeff Law
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2017-03-23 16:08 UTC (permalink / raw)
To: Richard Biener, gcc-patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]
Ping?
Best regards,
Thomas
On 16/03/17 14:05, Thomas Preudhomme wrote:
> Ping?
>
> Is this ok for stage4?
>
> Best regards,
>
> Thomas
>
> On 09/03/17 09:43, Thomas Preudhomme wrote:
>> Ping?
>>
>> Best regards,
>>
>> Thomas
>>
>> On 02/03/17 14:51, Thomas Preudhomme wrote:
>>> Hi,
>>>
>>> This patch fixes an assert failure when linking one LTOed object file
>>> having a weak alias with a regular object file containing a strong
>>> definition for that same symbol. The patch is twofold:
>>>
>>> + do not add an alias to a partition if it is external
>>> + do not declare (.globl) an alias if it is external
>>>
>>> ChangeLog entries are as follow:
>>>
>>> *** gcc/lto/ChangeLog ***
>>>
>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
>>> aliases to partition.
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
>>> declare external aliases.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * gcc.dg/lto/pr69866_0.c: New test.
>>> * gcc.dg/lto/pr69866_1.c: Likewise.
>>>
>>>
>>> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
>>> arm-none-eabi GCC cross-compiler, neither does it show any regression with
>>> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>>>
>>> Is this ok for stage4?
>>>
>>> Best regards,
>>>
>>> Thomas
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping2] Fix PR69866: LTO with def for weak alias in regular object file
2017-03-16 14:05 ` [PATCH, GCC/LTO, ping2] " Thomas Preudhomme
2017-03-23 16:08 ` [PATCH, GCC/LTO, stage4, ping3] " Thomas Preudhomme
@ 2017-03-31 15:25 ` Jeff Law
2017-03-31 18:11 ` Richard Biener
1 sibling, 1 reply; 15+ messages in thread
From: Jeff Law @ 2017-03-31 15:25 UTC (permalink / raw)
To: Thomas Preudhomme, Richard Biener, gcc-patches; +Cc: Jan Hubicka
On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
> Ping?
>
> Is this ok for stage4?
Given the lack of response from Richi, I'd suggest deferring to stage1.
jeff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping2] Fix PR69866: LTO with def for weak alias in regular object file
2017-03-31 15:25 ` [PATCH, GCC/LTO, ping2] " Jeff Law
@ 2017-03-31 18:11 ` Richard Biener
2017-05-02 16:56 ` [PATCH, GCC/LTO, ping3] " Thomas Preudhomme
0 siblings, 1 reply; 15+ messages in thread
From: Richard Biener @ 2017-03-31 18:11 UTC (permalink / raw)
To: Jeff Law, Thomas Preudhomme, gcc-patches; +Cc: Jan Hubicka
On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
>> Ping?
>>
>> Is this ok for stage4?
>Given the lack of response from Richi, I'd suggest deferring to stage1.
Honza needs to review this, i habe too little knowledge here.
Richard.
>jeff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping3] Fix PR69866: LTO with def for weak alias in regular object file
2017-03-31 18:11 ` Richard Biener
@ 2017-05-02 16:56 ` Thomas Preudhomme
2017-05-09 10:43 ` Thomas Preudhomme
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2017-05-02 16:56 UTC (permalink / raw)
To: gcc-patches, Jan Hubicka; +Cc: Richard Biener, Jeff Law
[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]
Now that GCC 7 is released, ping?
Original message below:
Hi,
This patch fixes an assert failure when linking one LTOed object file
having a weak alias with a regular object file containing a strong
definition for that same symbol. The patch is twofold:
+ do not add an alias to a partition if it is external
+ do not declare (.globl) an alias if it is external
ChangeLog entries are as follow:
*** gcc/lto/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
aliases to partition.
*** gcc/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
declare external aliases.
*** gcc/testsuite/ChangeLog ***
2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* gcc.dg/lto/pr69866_0.c: New test.
* gcc.dg/lto/pr69866_1.c: Likewise.
Testing: Testsuite shows no regression when targeting Cortex-M3 with an
arm-none-eabi GCC cross-compiler, neither does it show any regression with
native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
Is this ok for stage4?
Best regards,
Thomas
On 31/03/17 18:07, Richard Biener wrote:
> On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>> On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
>>> Ping?
>>>
>>> Is this ok for stage4?
>> Given the lack of response from Richi, I'd suggest deferring to stage1.
>
> Honza needs to review this, i habe too little knowledge here.
>
> Richard.
>
>> jeff
>
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping3] Fix PR69866: LTO with def for weak alias in regular object file
2017-05-02 16:56 ` [PATCH, GCC/LTO, ping3] " Thomas Preudhomme
@ 2017-05-09 10:43 ` Thomas Preudhomme
2017-05-09 23:52 ` Jan Hubicka
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2017-05-09 10:43 UTC (permalink / raw)
To: gcc-patches, Jan Hubicka; +Cc: Richard Biener, Jeff Law
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
Ping?
Best regards,
Thomas
On 02/05/17 17:52, Thomas Preudhomme wrote:
> Now that GCC 7 is released, ping?
>
> Original message below:
>
> Hi,
>
> This patch fixes an assert failure when linking one LTOed object file
> having a weak alias with a regular object file containing a strong
> definition for that same symbol. The patch is twofold:
>
> + do not add an alias to a partition if it is external
> + do not declare (.globl) an alias if it is external
>
> ChangeLog entries are as follow:
>
> *** gcc/lto/ChangeLog ***
>
> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> PR lto/69866
> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
> aliases to partition.
>
> *** gcc/ChangeLog ***
>
> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> PR lto/69866
> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
> declare external aliases.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> PR lto/69866
> * gcc.dg/lto/pr69866_0.c: New test.
> * gcc.dg/lto/pr69866_1.c: Likewise.
>
>
> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
> arm-none-eabi GCC cross-compiler, neither does it show any regression with
> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>
> Is this ok for stage4?
>
> Best regards,
>
> Thomas
>
> On 31/03/17 18:07, Richard Biener wrote:
>> On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>>> On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
>>>> Ping?
>>>>
>>>> Is this ok for stage4?
>>> Given the lack of response from Richi, I'd suggest deferring to stage1.
>>
>> Honza needs to review this, i habe too little knowledge here.
>>
>> Richard.
>>
>>> jeff
>>
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping3] Fix PR69866: LTO with def for weak alias in regular object file
2017-05-09 10:43 ` Thomas Preudhomme
@ 2017-05-09 23:52 ` Jan Hubicka
2017-05-10 10:57 ` Thomas Preudhomme
2017-06-06 10:12 ` Thomas Preudhomme
0 siblings, 2 replies; 15+ messages in thread
From: Jan Hubicka @ 2017-05-09 23:52 UTC (permalink / raw)
To: Thomas Preudhomme; +Cc: gcc-patches, Richard Biener, Jeff Law
> Ping?
Sorry for late reply
> >Hi,
> >
> >This patch fixes an assert failure when linking one LTOed object file
> >having a weak alias with a regular object file containing a strong
> >definition for that same symbol. The patch is twofold:
> >
> >+ do not add an alias to a partition if it is external
> >+ do not declare (.globl) an alias if it is external
Adding external alises to partitions is important to keep the information
that two symbols are the same.
The second part makes sense to me. What breaks when you drop the first
change?
Honza
> >
> >ChangeLog entries are as follow:
> >
> >*** gcc/lto/ChangeLog ***
> >
> >2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >
> > PR lto/69866
> > * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
> > aliases to partition.
> >
> >*** gcc/ChangeLog ***
> >
> >2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >
> > PR lto/69866
> > * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
> > declare external aliases.
> >
> >*** gcc/testsuite/ChangeLog ***
> >
> >2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >
> > PR lto/69866
> > * gcc.dg/lto/pr69866_0.c: New test.
> > * gcc.dg/lto/pr69866_1.c: Likewise.
> >
> >
> >Testing: Testsuite shows no regression when targeting Cortex-M3 with an
> >arm-none-eabi GCC cross-compiler, neither does it show any regression with
> >native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
> >
> >Is this ok for stage4?
> >
> >Best regards,
> >
> >Thomas
> >
> >On 31/03/17 18:07, Richard Biener wrote:
> >>On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
> >>>On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
> >>>>Ping?
> >>>>
> >>>>Is this ok for stage4?
> >>>Given the lack of response from Richi, I'd suggest deferring to stage1.
> >>
> >>Honza needs to review this, i habe too little knowledge here.
> >>
> >>Richard.
> >>
> >>>jeff
> >>
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
> FOR_EACH_ALIAS (this, ref)
> {
> cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
> - if (!alias->transparent_alias)
> + if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
> {
> bool saved_written = TREE_ASM_WRITTEN (decl);
>
> diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
> index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
> --- a/gcc/lto/lto-partition.c
> +++ b/gcc/lto/lto-partition.c
> @@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
> /* Add all aliases associated with the symbol. */
>
> FOR_EACH_ALIAS (node, ref)
> - if (!ref->referring->transparent_alias)
> + if (!ref->referring->transparent_alias
> + && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
> add_symbol_to_partition_1 (part, ref->referring);
> else
> {
> @@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
> {
> /* Nested transparent aliases are not permitted. */
> gcc_checking_assert (!ref2->referring->transparent_alias);
> - add_symbol_to_partition_1 (part, ref2->referring);
> + if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
> + add_symbol_to_partition_1 (part, ref2->referring);
> }
> }
>
> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> @@ -0,0 +1,13 @@
> +/* { dg-lto-do link } */
> +
> +int _umh(int i)
> +{
> + return i+1;
> +}
> +
> +int weaks(int i) __attribute__((weak, alias("_umh")));
> +
> +int main()
> +{
> + return weaks(10);
> +}
> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> @@ -0,0 +1,6 @@
> +/* { dg-options { -fno-lto } } */
> +
> +int weaks(int i)
> +{
> + return i+1;
> +}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping3] Fix PR69866: LTO with def for weak alias in regular object file
2017-05-09 23:52 ` Jan Hubicka
@ 2017-05-10 10:57 ` Thomas Preudhomme
2017-06-06 10:12 ` Thomas Preudhomme
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2017-05-10 10:57 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, Richard Biener, Jeff Law
Hi,
On 09/05/17 23:36, Jan Hubicka wrote:
>> Ping?
> Sorry for late reply
>>> Hi,
>>>
>>> This patch fixes an assert failure when linking one LTOed object file
>>> having a weak alias with a regular object file containing a strong
>>> definition for that same symbol. The patch is twofold:
>>>
>>> + do not add an alias to a partition if it is external
>>> + do not declare (.globl) an alias if it is external
>
> Adding external alises to partitions is important to keep the information
> that two symbols are the same.
> The second part makes sense to me. What breaks when you drop the first
> change?
Adding aliases to partitions is what cause the ICE referenced in the PR. It
fails on the following assert:
gcc_assert (c != SYMBOL_EXTERNAL
&& (c == SYMBOL_DUPLICATE || !symbol_partitioned_p (node)));
Second change came about when doing the first change because the linker was
complaining about the alias being defined twice (once in the trans object, once
in the non LTO object).
Best regards,
Thomas
>
> Honza
>>>
>>> ChangeLog entries are as follow:
>>>
>>> *** gcc/lto/ChangeLog ***
>>>
>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
>>> aliases to partition.
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
>>> declare external aliases.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * gcc.dg/lto/pr69866_0.c: New test.
>>> * gcc.dg/lto/pr69866_1.c: Likewise.
>>>
>>>
>>> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
>>> arm-none-eabi GCC cross-compiler, neither does it show any regression with
>>> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>>>
>>> Is this ok for stage4?
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>> On 31/03/17 18:07, Richard Biener wrote:
>>>> On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>>>>> On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
>>>>>> Ping?
>>>>>>
>>>>>> Is this ok for stage4?
>>>>> Given the lack of response from Richi, I'd suggest deferring to stage1.
>>>>
>>>> Honza needs to review this, i habe too little knowledge here.
>>>>
>>>> Richard.
>>>>
>>>>> jeff
>>>>
>
>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>> index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
>> --- a/gcc/cgraphunit.c
>> +++ b/gcc/cgraphunit.c
>> @@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
>> FOR_EACH_ALIAS (this, ref)
>> {
>> cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
>> - if (!alias->transparent_alias)
>> + if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
>> {
>> bool saved_written = TREE_ASM_WRITTEN (decl);
>>
>> diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
>> index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
>> --- a/gcc/lto/lto-partition.c
>> +++ b/gcc/lto/lto-partition.c
>> @@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
>> /* Add all aliases associated with the symbol. */
>>
>> FOR_EACH_ALIAS (node, ref)
>> - if (!ref->referring->transparent_alias)
>> + if (!ref->referring->transparent_alias
>> + && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
>> add_symbol_to_partition_1 (part, ref->referring);
>> else
>> {
>> @@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
>> {
>> /* Nested transparent aliases are not permitted. */
>> gcc_checking_assert (!ref2->referring->transparent_alias);
>> - add_symbol_to_partition_1 (part, ref2->referring);
>> + if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
>> + add_symbol_to_partition_1 (part, ref2->referring);
>> }
>> }
>>
>> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>> @@ -0,0 +1,13 @@
>> +/* { dg-lto-do link } */
>> +
>> +int _umh(int i)
>> +{
>> + return i+1;
>> +}
>> +
>> +int weaks(int i) __attribute__((weak, alias("_umh")));
>> +
>> +int main()
>> +{
>> + return weaks(10);
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>> @@ -0,0 +1,6 @@
>> +/* { dg-options { -fno-lto } } */
>> +
>> +int weaks(int i)
>> +{
>> + return i+1;
>> +}
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping3] Fix PR69866: LTO with def for weak alias in regular object file
2017-05-09 23:52 ` Jan Hubicka
2017-05-10 10:57 ` Thomas Preudhomme
@ 2017-06-06 10:12 ` Thomas Preudhomme
2017-06-12 9:13 ` [PATCH, GCC/LTO, ping] " Thomas Preudhomme
1 sibling, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2017-06-06 10:12 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, Richard Biener, Jeff Law
[-- Attachment #1: Type: text/plain, Size: 5545 bytes --]
On 09/05/17 23:36, Jan Hubicka wrote:
>> Ping?
> Sorry for late reply
My turn to apologize now.
>>> Hi,
>>>
>>> This patch fixes an assert failure when linking one LTOed object file
>>> having a weak alias with a regular object file containing a strong
>>> definition for that same symbol. The patch is twofold:
>>>
>>> + do not add an alias to a partition if it is external
>>> + do not declare (.globl) an alias if it is external
>
> Adding external alises to partitions is important to keep the information
> that two symbols are the same.
So how about simply relaxing the assert then? Right now it trips for any
external symbol, even external aliases.
How about the following:
ChangeLog entries are as follow:
*** gcc/lto/ChangeLog ***
2017-06-02 Thomas Preud'homme <thomas.preudhomme@arm.com>
* lto/lto-partition.c (add_symbol_to_partition_1): Change assert to
allow external aliases to be added.
*** gcc/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
* cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
declare external aliases.
*** gcc/testsuite/ChangeLog ***
2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
* gcc.dg/lto/pr69866_0.c: New test.
* gcc.dg/lto/pr69866_1.c: Likewise.
Bootstrapped with LTO on Aarch64 and ARM and testsuite on both of these
architectures do not show any regression.
Is this ok for trunk?
Best regards,
Thomas
> The second part makes sense to me. What breaks when you drop the first
> change?
>
> Honza
>>>
>>> ChangeLog entries are as follow:
>>>
>>> *** gcc/lto/ChangeLog ***
>>>
>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
>>> aliases to partition.
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
>>> declare external aliases.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> PR lto/69866
>>> * gcc.dg/lto/pr69866_0.c: New test.
>>> * gcc.dg/lto/pr69866_1.c: Likewise.
>>>
>>>
>>> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
>>> arm-none-eabi GCC cross-compiler, neither does it show any regression with
>>> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>>>
>>> Is this ok for stage4?
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>> On 31/03/17 18:07, Richard Biener wrote:
>>>> On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>>>>> On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
>>>>>> Ping?
>>>>>>
>>>>>> Is this ok for stage4?
>>>>> Given the lack of response from Richi, I'd suggest deferring to stage1.
>>>>
>>>> Honza needs to review this, i habe too little knowledge here.
>>>>
>>>> Richard.
>>>>
>>>>> jeff
>>>>
>
>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>> index c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d 100644
>> --- a/gcc/cgraphunit.c
>> +++ b/gcc/cgraphunit.c
>> @@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
>> FOR_EACH_ALIAS (this, ref)
>> {
>> cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
>> - if (!alias->transparent_alias)
>> + if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
>> {
>> bool saved_written = TREE_ASM_WRITTEN (decl);
>>
>> diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
>> index e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef 100644
>> --- a/gcc/lto/lto-partition.c
>> +++ b/gcc/lto/lto-partition.c
>> @@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
>> /* Add all aliases associated with the symbol. */
>>
>> FOR_EACH_ALIAS (node, ref)
>> - if (!ref->referring->transparent_alias)
>> + if (!ref->referring->transparent_alias
>> + && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
>> add_symbol_to_partition_1 (part, ref->referring);
>> else
>> {
>> @@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
>> {
>> /* Nested transparent aliases are not permitted. */
>> gcc_checking_assert (!ref2->referring->transparent_alias);
>> - add_symbol_to_partition_1 (part, ref2->referring);
>> + if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
>> + add_symbol_to_partition_1 (part, ref2->referring);
>> }
>> }
>>
>> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>> @@ -0,0 +1,13 @@
>> +/* { dg-lto-do link } */
>> +
>> +int _umh(int i)
>> +{
>> + return i+1;
>> +}
>> +
>> +int weaks(int i) __attribute__((weak, alias("_umh")));
>> +
>> +int main()
>> +{
>> + return weaks(10);
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>> @@ -0,0 +1,6 @@
>> +/* { dg-options { -fno-lto } } */
>> +
>> +int weaks(int i)
>> +{
>> + return i+1;
>> +}
>
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 1907 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 7b4f47e6efb41e7c401e7347d86fffca6618c4e9..c2cc9df6419573bdc6223e1d2ee348f8af69ccca 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1984,7 +1984,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 3600ab23bd9157aad76751912306abd498acae92..620deac090b7e718f05d3f37e6ef68e759de0008 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -132,7 +132,7 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Be sure that we never try to duplicate partitioned symbol
or add external symbol. */
- gcc_assert (c != SYMBOL_EXTERNAL
+ gcc_assert ((c != SYMBOL_EXTERNAL || node->alias)
&& (c == SYMBOL_DUPLICATE || !symbol_partitioned_p (node)));
part->symbols++;
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping] Fix PR69866: LTO with def for weak alias in regular object file
2017-06-06 10:12 ` Thomas Preudhomme
@ 2017-06-12 9:13 ` Thomas Preudhomme
2017-06-15 9:26 ` Jan Hubicka
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2017-06-12 9:13 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, Richard Biener, Jeff Law
[-- Attachment #1: Type: text/plain, Size: 5919 bytes --]
Ping?
Best regards,
Thomas
On 06/06/17 11:12, Thomas Preudhomme wrote:
> On 09/05/17 23:36, Jan Hubicka wrote:
>>> Ping?
>> Sorry for late reply
>
> My turn to apologize now.
>
>>>> Hi,
>>>>
>>>> This patch fixes an assert failure when linking one LTOed object file
>>>> having a weak alias with a regular object file containing a strong
>>>> definition for that same symbol. The patch is twofold:
>>>>
>>>> + do not add an alias to a partition if it is external
>>>> + do not declare (.globl) an alias if it is external
>>
>> Adding external alises to partitions is important to keep the information
>> that two symbols are the same.
>
> So how about simply relaxing the assert then? Right now it trips for any
> external symbol, even external aliases.
>
> How about the following:
>
>
> ChangeLog entries are as follow:
>
> *** gcc/lto/ChangeLog ***
>
> 2017-06-02 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * lto/lto-partition.c (add_symbol_to_partition_1): Change assert to
> allow external aliases to be added.
>
> *** gcc/ChangeLog ***
>
> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
> declare external aliases.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * gcc.dg/lto/pr69866_0.c: New test.
> * gcc.dg/lto/pr69866_1.c: Likewise.
>
> Bootstrapped with LTO on Aarch64 and ARM and testsuite on both of these
> architectures do not show any regression.
>
> Is this ok for trunk?
>
> Best regards,
>
> Thomas
>
>
>> The second part makes sense to me. What breaks when you drop the first
>> change?
>>
>> Honza
>>>>
>>>> ChangeLog entries are as follow:
>>>>
>>>> *** gcc/lto/ChangeLog ***
>>>>
>>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>> PR lto/69866
>>>> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
>>>> aliases to partition.
>>>>
>>>> *** gcc/ChangeLog ***
>>>>
>>>> 2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>> PR lto/69866
>>>> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
>>>> declare external aliases.
>>>>
>>>> *** gcc/testsuite/ChangeLog ***
>>>>
>>>> 2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>> PR lto/69866
>>>> * gcc.dg/lto/pr69866_0.c: New test.
>>>> * gcc.dg/lto/pr69866_1.c: Likewise.
>>>>
>>>>
>>>> Testing: Testsuite shows no regression when targeting Cortex-M3 with an
>>>> arm-none-eabi GCC cross-compiler, neither does it show any regression with
>>>> native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
>>>>
>>>> Is this ok for stage4?
>>>>
>>>> Best regards,
>>>>
>>>> Thomas
>>>>
>>>> On 31/03/17 18:07, Richard Biener wrote:
>>>>> On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>>>>>> On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
>>>>>>> Ping?
>>>>>>>
>>>>>>> Is this ok for stage4?
>>>>>> Given the lack of response from Richi, I'd suggest deferring to stage1.
>>>>>
>>>>> Honza needs to review this, i habe too little knowledge here.
>>>>>
>>>>> Richard.
>>>>>
>>>>>> jeff
>>>>>
>>
>>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>>> index
>>> c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d
>>> 100644
>>> --- a/gcc/cgraphunit.c
>>> +++ b/gcc/cgraphunit.c
>>> @@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
>>> FOR_EACH_ALIAS (this, ref)
>>> {
>>> cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
>>> - if (!alias->transparent_alias)
>>> + if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
>>> {
>>> bool saved_written = TREE_ASM_WRITTEN (decl);
>>>
>>> diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
>>> index
>>> e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef
>>> 100644
>>> --- a/gcc/lto/lto-partition.c
>>> +++ b/gcc/lto/lto-partition.c
>>> @@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part,
>>> symtab_node *node)
>>> /* Add all aliases associated with the symbol. */
>>>
>>> FOR_EACH_ALIAS (node, ref)
>>> - if (!ref->referring->transparent_alias)
>>> + if (!ref->referring->transparent_alias
>>> + && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
>>> add_symbol_to_partition_1 (part, ref->referring);
>>> else
>>> {
>>> @@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part,
>>> symtab_node *node)
>>> {
>>> /* Nested transparent aliases are not permitted. */
>>> gcc_checking_assert (!ref2->referring->transparent_alias);
>>> - add_symbol_to_partition_1 (part, ref2->referring);
>>> + if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
>>> + add_symbol_to_partition_1 (part, ref2->referring);
>>> }
>>> }
>>>
>>> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>>> b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
>>>
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
>>> @@ -0,0 +1,13 @@
>>> +/* { dg-lto-do link } */
>>> +
>>> +int _umh(int i)
>>> +{
>>> + return i+1;
>>> +}
>>> +
>>> +int weaks(int i) __attribute__((weak, alias("_umh")));
>>> +
>>> +int main()
>>> +{
>>> + return weaks(10);
>>> +}
>>> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>>> b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
>>>
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
>>> @@ -0,0 +1,6 @@
>>> +/* { dg-options { -fno-lto } } */
>>> +
>>> +int weaks(int i)
>>> +{
>>> + return i+1;
>>> +}
>>
[-- Attachment #2: fix_pr69866.patch --]
[-- Type: text/x-patch, Size: 1907 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 7b4f47e6efb41e7c401e7347d86fffca6618c4e9..c2cc9df6419573bdc6223e1d2ee348f8af69ccca 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1984,7 +1984,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 3600ab23bd9157aad76751912306abd498acae92..620deac090b7e718f05d3f37e6ef68e759de0008 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -132,7 +132,7 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Be sure that we never try to duplicate partitioned symbol
or add external symbol. */
- gcc_assert (c != SYMBOL_EXTERNAL
+ gcc_assert ((c != SYMBOL_EXTERNAL || node->alias)
&& (c == SYMBOL_DUPLICATE || !symbol_partitioned_p (node)));
part->symbols++;
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping] Fix PR69866: LTO with def for weak alias in regular object file
2017-06-12 9:13 ` [PATCH, GCC/LTO, ping] " Thomas Preudhomme
@ 2017-06-15 9:26 ` Jan Hubicka
0 siblings, 0 replies; 15+ messages in thread
From: Jan Hubicka @ 2017-06-15 9:26 UTC (permalink / raw)
To: Thomas Preudhomme; +Cc: gcc-patches, Richard Biener, Jeff Law
Hello,
I apologize for absurdly long time to check the patch. I misread the testcase
originally and got confused. Now it makes sense to me.
It does not make sense to keep the alias in symtab when we know that it
was prevailed by external definition. So lto-symtab should turn it into
normal external symbol in this case. There are multiple cases to consider
alias+target is interposed: in this case we probably want to keep them as
external alias to maintain the info that they might point to same address
but optimizer should take it as a may information.
alias is interposed, target is not: in this case we want to reset alias
into undefined state
alias is not interposed, target is: in this case we need to preserve target
definition but we also want to keep the extenral symbol. This will be bit
tricky to do becuase LTO lacks mechanizm to keep the body around.
I will fix the middle case first and look into other two (which are LTO-symtab
problem I know of for a while, but never got around fixing them)
Thanks and sorry for the slowness!
Honza
> Ping?
>
> Best regards,
>
> Thomas
>
> On 06/06/17 11:12, Thomas Preudhomme wrote:
> >On 09/05/17 23:36, Jan Hubicka wrote:
> >>>Ping?
> >>Sorry for late reply
> >
> >My turn to apologize now.
> >
> >>>>Hi,
> >>>>
> >>>>This patch fixes an assert failure when linking one LTOed object file
> >>>>having a weak alias with a regular object file containing a strong
> >>>>definition for that same symbol. The patch is twofold:
> >>>>
> >>>>+ do not add an alias to a partition if it is external
> >>>>+ do not declare (.globl) an alias if it is external
> >>
> >>Adding external alises to partitions is important to keep the information
> >>that two symbols are the same.
> >
> >So how about simply relaxing the assert then? Right now it trips for any
> >external symbol, even external aliases.
> >
> >How about the following:
> >
> >
> >ChangeLog entries are as follow:
> >
> >*** gcc/lto/ChangeLog ***
> >
> >2017-06-02 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >
> > * lto/lto-partition.c (add_symbol_to_partition_1): Change assert to
> > allow external aliases to be added.
> >
> >*** gcc/ChangeLog ***
> >
> >2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >
> > * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
> > declare external aliases.
> >
> >*** gcc/testsuite/ChangeLog ***
> >
> >2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >
> > * gcc.dg/lto/pr69866_0.c: New test.
> > * gcc.dg/lto/pr69866_1.c: Likewise.
> >
> >Bootstrapped with LTO on Aarch64 and ARM and testsuite on both of these
> >architectures do not show any regression.
> >
> >Is this ok for trunk?
> >
> >Best regards,
> >
> >Thomas
> >
> >
> >>The second part makes sense to me. What breaks when you drop the first
> >>change?
> >>
> >>Honza
> >>>>
> >>>>ChangeLog entries are as follow:
> >>>>
> >>>>*** gcc/lto/ChangeLog ***
> >>>>
> >>>>2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >>>>
> >>>> PR lto/69866
> >>>> * lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
> >>>> aliases to partition.
> >>>>
> >>>>*** gcc/ChangeLog ***
> >>>>
> >>>>2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >>>>
> >>>> PR lto/69866
> >>>> * cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
> >>>> declare external aliases.
> >>>>
> >>>>*** gcc/testsuite/ChangeLog ***
> >>>>
> >>>>2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
> >>>>
> >>>> PR lto/69866
> >>>> * gcc.dg/lto/pr69866_0.c: New test.
> >>>> * gcc.dg/lto/pr69866_1.c: Likewise.
> >>>>
> >>>>
> >>>>Testing: Testsuite shows no regression when targeting Cortex-M3 with an
> >>>>arm-none-eabi GCC cross-compiler, neither does it show any regression with
> >>>>native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
> >>>>
> >>>>Is this ok for stage4?
> >>>>
> >>>>Best regards,
> >>>>
> >>>>Thomas
> >>>>
> >>>>On 31/03/17 18:07, Richard Biener wrote:
> >>>>>On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
> >>>>>>On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
> >>>>>>>Ping?
> >>>>>>>
> >>>>>>>Is this ok for stage4?
> >>>>>>Given the lack of response from Richi, I'd suggest deferring to stage1.
> >>>>>
> >>>>>Honza needs to review this, i habe too little knowledge here.
> >>>>>
> >>>>>Richard.
> >>>>>
> >>>>>>jeff
> >>>>>
> >>
> >>>diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> >>>index
> >>>c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d
> >>>100644
> >>>--- a/gcc/cgraphunit.c
> >>>+++ b/gcc/cgraphunit.c
> >>>@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
> >>> FOR_EACH_ALIAS (this, ref)
> >>> {
> >>> cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
> >>>- if (!alias->transparent_alias)
> >>>+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
> >>> {
> >>> bool saved_written = TREE_ASM_WRITTEN (decl);
> >>>
> >>>diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
> >>>index
> >>>e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef
> >>>100644
> >>>--- a/gcc/lto/lto-partition.c
> >>>+++ b/gcc/lto/lto-partition.c
> >>>@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part,
> >>>symtab_node *node)
> >>> /* Add all aliases associated with the symbol. */
> >>>
> >>> FOR_EACH_ALIAS (node, ref)
> >>>- if (!ref->referring->transparent_alias)
> >>>+ if (!ref->referring->transparent_alias
> >>>+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
> >>> add_symbol_to_partition_1 (part, ref->referring);
> >>> else
> >>> {
> >>>@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part,
> >>>symtab_node *node)
> >>> {
> >>> /* Nested transparent aliases are not permitted. */
> >>> gcc_checking_assert (!ref2->referring->transparent_alias);
> >>>- add_symbol_to_partition_1 (part, ref2->referring);
> >>>+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
> >>>+ add_symbol_to_partition_1 (part, ref2->referring);
> >>> }
> >>> }
> >>>
> >>>diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> >>>b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> >>>new file mode 100644
> >>>index
> >>>0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
> >>>
> >>>--- /dev/null
> >>>+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> >>>@@ -0,0 +1,13 @@
> >>>+/* { dg-lto-do link } */
> >>>+
> >>>+int _umh(int i)
> >>>+{
> >>>+ return i+1;
> >>>+}
> >>>+
> >>>+int weaks(int i) __attribute__((weak, alias("_umh")));
> >>>+
> >>>+int main()
> >>>+{
> >>>+ return weaks(10);
> >>>+}
> >>>diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> >>>b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> >>>new file mode 100644
> >>>index
> >>>0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
> >>>
> >>>--- /dev/null
> >>>+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> >>>@@ -0,0 +1,6 @@
> >>>+/* { dg-options { -fno-lto } } */
> >>>+
> >>>+int weaks(int i)
> >>>+{
> >>>+ return i+1;
> >>>+}
> >>
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 7b4f47e6efb41e7c401e7347d86fffca6618c4e9..c2cc9df6419573bdc6223e1d2ee348f8af69ccca 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1984,7 +1984,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
> FOR_EACH_ALIAS (this, ref)
> {
> cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
> - if (!alias->transparent_alias)
> + if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
> {
> bool saved_written = TREE_ASM_WRITTEN (decl);
>
> diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
> index 3600ab23bd9157aad76751912306abd498acae92..620deac090b7e718f05d3f37e6ef68e759de0008 100644
> --- a/gcc/lto/lto-partition.c
> +++ b/gcc/lto/lto-partition.c
> @@ -132,7 +132,7 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
>
> /* Be sure that we never try to duplicate partitioned symbol
> or add external symbol. */
> - gcc_assert (c != SYMBOL_EXTERNAL
> + gcc_assert ((c != SYMBOL_EXTERNAL || node->alias)
> && (c == SYMBOL_DUPLICATE || !symbol_partitioned_p (node)));
>
> part->symbols++;
> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
> @@ -0,0 +1,13 @@
> +/* { dg-lto-do link } */
> +
> +int _umh(int i)
> +{
> + return i+1;
> +}
> +
> +int weaks(int i) __attribute__((weak, alias("_umh")));
> +
> +int main()
> +{
> + return weaks(10);
> +}
> diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
> @@ -0,0 +1,6 @@
> +/* { dg-options { -fno-lto } } */
> +
> +int weaks(int i)
> +{
> + return i+1;
> +}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping] Fix PR69866: LTO with def for weak alias in regular object file
2017-06-17 16:32 Dominique d'Humières
@ 2017-06-18 19:56 ` Jan Hubicka
0 siblings, 0 replies; 15+ messages in thread
From: Jan Hubicka @ 2017-06-18 19:56 UTC (permalink / raw)
To: Dominique d'Humi??res; +Cc: hubicka Jan, gcc-patches
> The new test fails on darwin with the usual
>
> FAIL: gcc.dg/lto/pr69866 c_lto_pr69866_0.o-c_lto_pr69866_1.o link, -O0 -flto -flto-partition=none
>
> IMO it requires a
>
> /* { dg-require-alias "" } */
Yep,I will add it shortly.
Honza
>
> directive.
>
> TIA
>
> Dominique
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, GCC/LTO, ping] Fix PR69866: LTO with def for weak alias in regular object file
@ 2017-06-17 16:32 Dominique d'Humières
2017-06-18 19:56 ` Jan Hubicka
0 siblings, 1 reply; 15+ messages in thread
From: Dominique d'Humières @ 2017-06-17 16:32 UTC (permalink / raw)
To: hubicka Jan; +Cc: gcc-patches
The new test fails on darwin with the usual
FAIL: gcc.dg/lto/pr69866 c_lto_pr69866_0.o-c_lto_pr69866_1.o link, -O0 -flto -flto-partition=none
IMO it requires a
/* { dg-require-alias "" } */
directive.
TIA
Dominique
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-06-18 19:56 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-02 14:52 [PATCH, GCC/LTO] Fix PR69866: LTO with def for weak alias in regular object file Thomas Preudhomme
2017-03-09 9:43 ` Thomas Preudhomme
2017-03-16 14:05 ` [PATCH, GCC/LTO, ping2] " Thomas Preudhomme
2017-03-23 16:08 ` [PATCH, GCC/LTO, stage4, ping3] " Thomas Preudhomme
2017-03-31 15:25 ` [PATCH, GCC/LTO, ping2] " Jeff Law
2017-03-31 18:11 ` Richard Biener
2017-05-02 16:56 ` [PATCH, GCC/LTO, ping3] " Thomas Preudhomme
2017-05-09 10:43 ` Thomas Preudhomme
2017-05-09 23:52 ` Jan Hubicka
2017-05-10 10:57 ` Thomas Preudhomme
2017-06-06 10:12 ` Thomas Preudhomme
2017-06-12 9:13 ` [PATCH, GCC/LTO, ping] " Thomas Preudhomme
2017-06-15 9:26 ` Jan Hubicka
2017-06-17 16:32 Dominique d'Humières
2017-06-18 19:56 ` Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).