From patchwork Thu Jul 18 17:05:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Vladimir Makarov X-Patchwork-Id: 260100 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id CF2FA2C063C for ; Fri, 19 Jul 2013 03:05:57 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=UvWxFEcYOKq/qleMj i8yo9+hETRUfd6HJNkszhWjR2tLYUFIUUke5xk+EUfA694RTiz2RI5PrzyyNYdAu 8gH0q8vcchBmfwKBr/9XZIKPmyW6nxhAKp2La4Idyt+HT/tX5WGo5h/KEPmXVxOS oN+Sv0ZQi/Cr5wwgIQilXSUQlM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=vocbUguc8s8JHbu4xyapEIK 1juU=; b=CQl5XEpktitMlOtZCi6xiQtEG8vxu2NwfOPWB8uVaU2Kn0mXVKsVQSP E6dVdPb3rMgFSy983WnossPIzC1XrCwx5NgkMBBgi42rDWlYpk37n0C7OwsJbME/ SpO84HoP+ERNh3AdmY+thPoWYAdepfQ20jFKOxFaK4DAwp37SfGk= Received: (qmail 19197 invoked by alias); 18 Jul 2013 17:05:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 19171 invoked by uid 89); 18 Jul 2013 17:05:50 -0000 X-Spam-SWARE-Status: No, score=-6.1 required=5.0 tests=AWL, BAYES_05, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RDNS_NONE, SPF_HELO_PASS, SPF_PASS, TW_CX autolearn=no version=3.3.1 Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 18 Jul 2013 17:05:49 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6IH5fh5012025 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 18 Jul 2013 13:05:42 -0400 Received: from topor.usersys.redhat.com ([10.15.16.142]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r6IH5f17018702; Thu, 18 Jul 2013 13:05:41 -0400 Message-ID: <51E82064.8020704@redhat.com> Date: Thu, 18 Jul 2013 13:05:40 -0400 From: Vladimir Makarov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Wei Mi CC: GCC Patches , Easwaran Raman Subject: Re: [PATCH] PR57878, Incorrect code: live register clobbered in split2 References: In-Reply-To: X-Virus-Found: No On 07/15/2013 02:26 PM, Wei Mi wrote: > Hi, > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57878 > > The bug occurs because tfreq is given higher priority than bigger mode > in reload_pseudo_compare_func. When there are multiple reload pseudos > in the same insn， and the pseudo with bigger mode has lower thread > frequency than other reload pseudos, it is possible the bigger mode > pseudo cannot find available hardregs. > > The proposed fix is to switch the priority of bigger mode and tfreq in > reload_pseudo_compare_func. Besides I promoted lra_assert to > gcc_assert at the end of to make adding testcase easier since > lra_assert will not fire on 4.8.1. > > bootstrap and regression test are ok on x86_64-linux-gnu. Is it ok for > trunk and 4.8 branch? > > Thanks, > Wei. > > 2013-07-15 Wei Mi > > PR rtl-optimization/57878 > * lra-assigns.c (reload_pseudo_compare_func): Switch the priority of > bigger mode and tfreq. > > 2013-07-15 Wei Mi > > PR rtl-optimization/57878 > * g++.dg/pr57518.C: New test. In overall, the PR analysis and the patch is ok. The only problem the patch can affect generated code performance especially for 32-bit targets. I see regressions on 4 SPECInt2000 tests. Here is the patch I've committed into the trunk. It decreases the patch effect on performance. The patch was successfully bootstrapped an tested on x86/x86-64. Committed to the trunk as rev. 201036. Wei Mi, could you commit it to gcc4.8 branch. Thanks. 2013-07-18 Vladimir Makarov Wei Mi PR rtl-optimization/57878 * lra-assigns.c (assign_by_spills): Move non_reload_pseudos to the top. (reload_pseudo_compare_func): Check nregs first for reload pseudos. 2013-07-18 Wei Mi PR rtl-optimization/57878 * g++.dg/pr57518.C: New test. Index: lra-assigns.c =================================================================== --- lra-assigns.c (revision 200959) +++ lra-assigns.c (working copy) @@ -116,6 +116,11 @@ struct regno_assign_info /* Map regno to the corresponding regno assignment info. */ static struct regno_assign_info *regno_assign_info; +/* All inherited, subreg or optional pseudos created before last spill + sub-pass. Such pseudos are permitted to get memory instead of hard + regs. */ +static bitmap_head non_reload_pseudos; + /* Process a pseudo copy with execution frequency COPY_FREQ connecting REGNO1 and REGNO2 to form threads. */ static void @@ -194,6 +199,15 @@ reload_pseudo_compare_func (const void * if ((diff = (ira_class_hard_regs_num[cl1] - ira_class_hard_regs_num[cl2])) != 0) return diff; + if ((diff + = (ira_reg_class_max_nregs[cl2][lra_reg_info[r2].biggest_mode] + - ira_reg_class_max_nregs[cl1][lra_reg_info[r1].biggest_mode])) != 0 + /* The code below executes rarely as nregs == 1 in most cases. + So we should not worry about using faster data structures to + check reload pseudos. */ + && ! bitmap_bit_p (&non_reload_pseudos, r1) + && ! bitmap_bit_p (&non_reload_pseudos, r2)) + return diff; if ((diff = (regno_assign_info[regno_assign_info[r2].first].freq - regno_assign_info[regno_assign_info[r1].first].freq)) != 0) return diff; @@ -1155,7 +1169,6 @@ assign_by_spills (void) rtx insn; basic_block bb; bitmap_head changed_insns, do_not_assign_nonreload_pseudos; - bitmap_head non_reload_pseudos; unsigned int u; bitmap_iterator bi; bool reload_p; Index: testsuite/g++.dg/pr57878.C =================================================================== --- testsuite/g++.dg/pr57878.C (revision 0) +++ testsuite/g++.dg/pr57878.C (working copy) @@ -0,0 +1,226 @@ +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ +/* { dg-options "-m32 -O2 -fno-omit-frame-pointer -fPIC -std=gnu++11" } */ + +typedef int int32; +typedef long long int64; +typedef unsigned int uint32; +typedef unsigned long long uint64; +namespace std { + typedef unsigned int size_t; + template + struct char_traits; + template + inline _Tp* __addressof(_Tp& __r) noexcept { + return reinterpret_cast<_Tp*> (&const_cast(reinterpret_cast(__r))); + } + template + struct remove_reference { + typedef _Tp type; + }; + template + constexpr _Tp&& forward(typename std::remove_reference<_Tp>::type& __t) noexcept { + return static_cast<_Tp&&>(__t); + } +} +typedef unsigned int size_t; +extern "C++" { + inline void* operator new(std::size_t, void* __p) noexcept { + return __p; + } +} +namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) { + template + class new_allocator { + public: + typedef size_t size_type; + typedef _Tp* pointer; + }; +} +namespace std { + template + using __allocator_base = __gnu_cxx::new_allocator<_Tp>; + template + class allocator + : public __allocator_base<_Tp> { + public: + typedef size_t size_type; + template + struct rebind { + typedef allocator<_Tp1> other; + }; + }; +} +namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) { + template + class __sso_string_base; + template, typename _Alloc = std::allocator<_CharT>, template class _Base = __sso_string_base> + class __versa_string; + template + struct __vstring_utility { + typedef typename _Alloc::template rebind<_CharT>::other _CharT_alloc_type; + template + struct _Alloc_hider + : public _Alloc1 { + _Alloc_hider(const _Alloc1& __a, _CharT* __ptr) + : _Alloc1(__a), _M_p(__ptr) { + } + _CharT* _M_p; + }; + }; + template + class __sso_string_base + : protected __vstring_utility<_CharT, _Traits, _Alloc> { + typedef __vstring_utility<_CharT, _Traits, _Alloc> _Util_Base; + typedef typename _Util_Base::_CharT_alloc_type _CharT_alloc_type; + typedef typename _CharT_alloc_type::size_type size_type; + private: + typename _Util_Base::template _Alloc_hider<_CharT_alloc_type> + _M_dataplus; + size_type _M_string_length; + enum { + _S_local_capacity = 15 }; + union { + _CharT _M_local_data[_S_local_capacity + 1]; + }; + template + void _M_construct(_InIterator __beg, _InIterator __end); + public: + size_type _M_max_size() const; + _CharT* _M_data() const { + return _M_dataplus._M_p; + } + size_type _M_length() const { + return _M_string_length; + } + __sso_string_base(const __sso_string_base& __rcs); + const _CharT_alloc_type& _M_get_allocator() const { + } + }; + template + __sso_string_base<_CharT, _Traits, _Alloc>:: __sso_string_base(const __sso_string_base& __rcs) + : _M_dataplus(__rcs._M_get_allocator(), _M_local_data) { + _M_construct(__rcs._M_data(), __rcs._M_data() + __rcs._M_length()); + } + template class _Base> + class __versa_string + : private _Base<_CharT, _Traits, _Alloc> { + }; +} +template, typename _Alloc = std::allocator<_CharT> > +class basic_string + : public __gnu_cxx::__versa_string<_CharT, _Traits, _Alloc> { +}; +typedef basic_string string; +namespace std __attribute__ ((__visibility__ ("default"))) { + template + class __alloctr_rebind_helper { + public: + static const bool __value = true; + }; + template::__value> + struct __alloctr_rebind; + template struct __alloctr_rebind<_Alloc, _Tp, true> + { + typedef typename _Alloc::template rebind<_Tp>::other __type; + }; + template + struct allocator_traits { + private: + template + static typename _Tp::pointer _S_pointer_helper(_Tp*); + typedef decltype(_S_pointer_helper((_Alloc*)0)) __pointer; + public: + typedef __pointer pointer; + template + using rebind_alloc = typename __alloctr_rebind<_Alloc, _Tp>::__type; + }; +} +namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) { + template struct __alloc_traits + : std::allocator_traits<_Alloc> + { + typedef std::allocator_traits<_Alloc> _Base_type; + template + struct rebind { + typedef typename _Base_type::template rebind_alloc<_Tp> + other; + }; + }; +} +namespace std __attribute__ ((__visibility__ ("default"))) { + template + inline void _Construct(_T1* __p, _Args&&... __args) { + ::new(static_cast(__p)) _T1(std::forward<_Args>(__args)...); + } + template + struct _Vector_base { + typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template rebind<_Tp>::other _Tp_alloc_type; + typedef typename __gnu_cxx::__alloc_traits<_Tp_alloc_type>::pointer pointer; + struct _Vector_impl + : public _Tp_alloc_type { + pointer _M_start; + pointer _M_finish; + }; + public: + _Vector_impl _M_impl; + }; + template > + class vector + : protected _Vector_base<_Tp, _Alloc> { + typedef _Vector_base<_Tp, _Alloc> _Base; + public: + typedef _Tp value_type; + typedef typename _Base::pointer pointer; + typedef size_t size_type; + size_type size() const; + void push_back(const value_type& __x) { + _M_emplace_back_aux(__x); + } + template + void _M_emplace_back_aux(_Args&&... __args); + size_type _M_check_len(); + }; + template template + void vector<_Tp, _Alloc>:: _M_emplace_back_aux(_Args&&... __args) { + const size_type __len = _M_check_len(); + pointer __new_start(static_cast(::operator new(__len * sizeof(_Tp)))); + pointer __new_temp(__new_start + size()); + ::new((void *)__new_temp) _Tp(std::forward<_Args>(__args)...); + pointer __cur = __new_start; + pointer __first = this->_M_impl._M_start; + pointer __last = this->_M_impl._M_finish; + for (; + __first != __last; + ++__first, ++__cur) std::_Construct(std::__addressof(*__cur), *__first); + } +} +using std::vector; +class DL { +public: + struct ChunkId { + int64 disk_id; + uint64 handle; + uint64 version; + string capability; + ChunkId(); + }; + struct ChunkInfo { + ChunkId id; + uint64 mtime; + uint32 length; + int32 space_used; + }; +}; +class FDB { + void CollectChunk(const DL::ChunkInfo& chunk, const int& location); +private: + struct ChunkData { + int location; + DL::ChunkInfo chunk_info; + }; + vector chunk_data_; +}; +void FDB::CollectChunk(const DL::ChunkInfo& chunk, const int& location) { + ChunkData chunk_data; + chunk_data_.push_back( chunk_data); +}