diff mbox

PR rtl-optimization/32219: optimizer causees wrong code in pic/hidden/weak symbol checking

Message ID CAFULd4bsDPrsv+SObbmYDU-a8piwxcZ5LMqYfPrXQBMnQ95jzw@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Feb. 19, 2015, 9:04 p.m. UTC
On Mon, Feb 16, 2015 at 5:30 PM, Richard Henderson <rth@redhat.com> wrote:

>>>> 2015-02-12  H.J. Lu  <hongjiu.lu@intel.com>
>>>>             Richard Henderson  <rth@redhat.com>
>>>>
>>>>         PR rtl/32219
>>>>         * cgraphunit.c (cgraph_node::finalize_function): Set definition
>>>>         before notice_global_symbol.
>>>>         (varpool_node::finalize_decl): Likewise.
>>>>         * varasm.c (default_binds_local_p_2): Rename from
>>>>         default_binds_local_p_1, add weak_dominate argument.  Use direct
>>>>         returns instead of assigning to local variable.  Unify varpool and
>>>>         cgraph paths via symtab_node.  Reject undef weak variables before
>>>>         testing visibility.  Reorder tests for simplicity.
>>>>         (default_binds_local_p): Use default_binds_local_p_2.
>>>>         (default_binds_local_p_1): Likewise.
>>>>         (decl_binds_to_current_def_p): Unify varpool and cgraph paths
>>>>         via symtab_node.
>>>>         (default_elf_asm_output_external): Emit visibility when specified.
>>>
>>> It looks like this patch broke alphaev68-linux-gnu [1]. There are many
>>> failures of the type:
>>>
>>> /tmp/cck7V7MR.o: In function
>>> `__static_initialization_and_destruction_0(int, int)':^M
>>> (.text+0x3ac): relocation truncated to fit: GPRELHIGH against symbol
>>> `std::__cxx11::basic_string<char, std::char_traits<char>,
>>> std::allocator<char> >::~basic_string()@@GLIBCXX_3.4.21' defined in
>>> .text section in
>>> /space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libstdc++-v3/src/.libs/libstdc++.so^M
>>> /space/homedirs/uros/local/bin/ld: /tmp/cck7V7MR.o: gp-relative
>>> relocation against dynamic symbol
>>
>> It could be related to:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65064
>>
>> Before this bug fix, all common symbols don't bind locally,
>> which is one of PR 32219 bugs.  After this fix, common
>> symbols bind locally.  It may cause problems on targets with
>> small data sections and common symbols aren't in small
>> data section:
>
> This is a destructor, and so obviously not a common symbol.
>
> I'll have a look.

The attached patch fixes all alpha-linux-gnu failures.

2015-02-19  Uros Bizjak  <ubizjak@gmail.com>

    * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.

Patch was bootstrapped and regression tested on alphaev68-linux-gnu.

OK for mainline?

Uros.

Comments

H.J. Lu Feb. 19, 2015, 9:07 p.m. UTC | #1
On Thu, Feb 19, 2015 at 1:04 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Feb 16, 2015 at 5:30 PM, Richard Henderson <rth@redhat.com> wrote:
>
>>>>> 2015-02-12  H.J. Lu  <hongjiu.lu@intel.com>
>>>>>             Richard Henderson  <rth@redhat.com>
>>>>>
>>>>>         PR rtl/32219
>>>>>         * cgraphunit.c (cgraph_node::finalize_function): Set definition
>>>>>         before notice_global_symbol.
>>>>>         (varpool_node::finalize_decl): Likewise.
>>>>>         * varasm.c (default_binds_local_p_2): Rename from
>>>>>         default_binds_local_p_1, add weak_dominate argument.  Use direct
>>>>>         returns instead of assigning to local variable.  Unify varpool and
>>>>>         cgraph paths via symtab_node.  Reject undef weak variables before
>>>>>         testing visibility.  Reorder tests for simplicity.
>>>>>         (default_binds_local_p): Use default_binds_local_p_2.
>>>>>         (default_binds_local_p_1): Likewise.
>>>>>         (decl_binds_to_current_def_p): Unify varpool and cgraph paths
>>>>>         via symtab_node.
>>>>>         (default_elf_asm_output_external): Emit visibility when specified.
>>>>
>>>> It looks like this patch broke alphaev68-linux-gnu [1]. There are many
>>>> failures of the type:
>>>>
>>>> /tmp/cck7V7MR.o: In function
>>>> `__static_initialization_and_destruction_0(int, int)':^M
>>>> (.text+0x3ac): relocation truncated to fit: GPRELHIGH against symbol
>>>> `std::__cxx11::basic_string<char, std::char_traits<char>,
>>>> std::allocator<char> >::~basic_string()@@GLIBCXX_3.4.21' defined in
>>>> .text section in
>>>> /space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libstdc++-v3/src/.libs/libstdc++.so^M
>>>> /space/homedirs/uros/local/bin/ld: /tmp/cck7V7MR.o: gp-relative
>>>> relocation against dynamic symbol
>>>
>>> It could be related to:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65064
>>>
>>> Before this bug fix, all common symbols don't bind locally,
>>> which is one of PR 32219 bugs.  After this fix, common
>>> symbols bind locally.  It may cause problems on targets with
>>> small data sections and common symbols aren't in small
>>> data section:
>>
>> This is a destructor, and so obviously not a common symbol.
>>
>> I'll have a look.
>
> The attached patch fixes all alpha-linux-gnu failures.
>
> 2015-02-19  Uros Bizjak  <ubizjak@gmail.com>
>
>     * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.
>
> Patch was bootstrapped and regression tested on alphaev68-linux-gnu.
>
> OK for mainline?
>

You may want to use something like this:

https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01105.html
H.J. Lu Feb. 19, 2015, 9:08 p.m. UTC | #2
On Thu, Feb 19, 2015 at 1:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Feb 19, 2015 at 1:04 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Mon, Feb 16, 2015 at 5:30 PM, Richard Henderson <rth@redhat.com> wrote:
>>
>>>>>> 2015-02-12  H.J. Lu  <hongjiu.lu@intel.com>
>>>>>>             Richard Henderson  <rth@redhat.com>
>>>>>>
>>>>>>         PR rtl/32219
>>>>>>         * cgraphunit.c (cgraph_node::finalize_function): Set definition
>>>>>>         before notice_global_symbol.
>>>>>>         (varpool_node::finalize_decl): Likewise.
>>>>>>         * varasm.c (default_binds_local_p_2): Rename from
>>>>>>         default_binds_local_p_1, add weak_dominate argument.  Use direct
>>>>>>         returns instead of assigning to local variable.  Unify varpool and
>>>>>>         cgraph paths via symtab_node.  Reject undef weak variables before
>>>>>>         testing visibility.  Reorder tests for simplicity.
>>>>>>         (default_binds_local_p): Use default_binds_local_p_2.
>>>>>>         (default_binds_local_p_1): Likewise.
>>>>>>         (decl_binds_to_current_def_p): Unify varpool and cgraph paths
>>>>>>         via symtab_node.
>>>>>>         (default_elf_asm_output_external): Emit visibility when specified.
>>>>>
>>>>> It looks like this patch broke alphaev68-linux-gnu [1]. There are many
>>>>> failures of the type:
>>>>>
>>>>> /tmp/cck7V7MR.o: In function
>>>>> `__static_initialization_and_destruction_0(int, int)':^M
>>>>> (.text+0x3ac): relocation truncated to fit: GPRELHIGH against symbol
>>>>> `std::__cxx11::basic_string<char, std::char_traits<char>,
>>>>> std::allocator<char> >::~basic_string()@@GLIBCXX_3.4.21' defined in
>>>>> .text section in
>>>>> /space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libstdc++-v3/src/.libs/libstdc++.so^M
>>>>> /space/homedirs/uros/local/bin/ld: /tmp/cck7V7MR.o: gp-relative
>>>>> relocation against dynamic symbol
>>>>
>>>> It could be related to:
>>>>
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65064
>>>>
>>>> Before this bug fix, all common symbols don't bind locally,
>>>> which is one of PR 32219 bugs.  After this fix, common
>>>> symbols bind locally.  It may cause problems on targets with
>>>> small data sections and common symbols aren't in small
>>>> data section:
>>>
>>> This is a destructor, and so obviously not a common symbol.
>>>
>>> I'll have a look.
>>
>> The attached patch fixes all alpha-linux-gnu failures.
>>
>> 2015-02-19  Uros Bizjak  <ubizjak@gmail.com>
>>
>>     * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.
>>
>> Patch was bootstrapped and regression tested on alphaev68-linux-gnu.
>>
>> OK for mainline?
>>
>
> You may want to use something like this:
>
> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01105.html
>

See:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65064

for why.
Richard Henderson Feb. 19, 2015, 9:16 p.m. UTC | #3
On 02/19/2015 01:07 PM, H.J. Lu wrote:
>> > The attached patch fixes all alpha-linux-gnu failures.
>> >
>> > 2015-02-19  Uros Bizjak  <ubizjak@gmail.com>
>> >
>> >     * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.
>> >
>> > Patch was bootstrapped and regression tested on alphaev68-linux-gnu.
>> >
>> > OK for mainline?
>> >
> You may want to use something like this:
> 
> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01105.html

That's exactly what he's done.


r~
Richard Henderson Feb. 19, 2015, 9:16 p.m. UTC | #4
On 02/19/2015 01:04 PM, Uros Bizjak wrote:
> 2015-02-19  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.

Ok.  Thanks.


r~
H.J. Lu Feb. 19, 2015, 9:35 p.m. UTC | #5
On Thu, Feb 19, 2015 at 1:16 PM, Richard Henderson <rth@redhat.com> wrote:
> On 02/19/2015 01:04 PM, Uros Bizjak wrote:
>> 2015-02-19  Uros Bizjak  <ubizjak@gmail.com>
>>
>>     * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.
>
> Ok.  Thanks.

We tried it on ia64 and failed with Ada build.
Uros Bizjak Feb. 19, 2015, 9:43 p.m. UTC | #6
On Thu, Feb 19, 2015 at 10:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Feb 19, 2015 at 1:16 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 02/19/2015 01:04 PM, Uros Bizjak wrote:
>>> 2015-02-19  Uros Bizjak  <ubizjak@gmail.com>
>>>
>>>     * config/alpha/alpha.c (alpha_in_small_data_p): Reject common symbols.
>>
>> Ok.  Thanks.
>
> We tried it on ia64 and failed with Ada build.

As I see from the PR, it was the pilot error, please see comment #14 [1].

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65064#c14

Uros.
diff mbox

Patch

Index: config/alpha/alpha.c
===================================================================
--- config/alpha/alpha.c        (revision 220806)
+++ config/alpha/alpha.c        (working copy)
@@ -835,6 +835,10 @@  alpha_in_small_data_p (const_tree exp)
   if (TREE_CODE (exp) == FUNCTION_DECL)
     return false;

+  /* COMMON symbols are never small data.  */
+  if (TREE_CODE (exp) == VAR_DECL && DECL_COMMON (exp))
+    return false;
+
   if (TREE_CODE (exp) == VAR_DECL && DECL_SECTION_NAME (exp))
     {
       const char *section = DECL_SECTION_NAME (exp);