From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4193 invoked by alias); 16 Jun 2014 16:35:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4180 invoked by uid 89); 16 Jun 2014 16:35:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_00,RDNS_DYNAMIC,TVD_RCVD_IP autolearn=no version=3.3.2 X-HELO: brightrain.aerifal.cx Received: from 216-12-86-13.cv.mvl.ntelos.net (HELO brightrain.aerifal.cx) (216.12.86.13) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Jun 2014 16:35:28 +0000 Received: from dalias by brightrain.aerifal.cx with local (Exim 3.15 #2) id 1WwZsB-0006zL-00; Mon, 16 Jun 2014 16:35:07 +0000 Date: Mon, 16 Jun 2014 16:35:00 -0000 From: Rich Felker To: Jan Hubicka Cc: Alexander Monakov , Jeff Law , Richard Biener , GCC Patches Subject: Re: [PATCH] proposed fix for bug # 61144 Message-ID: <20140616163507.GD179@brightrain.aerifal.cx> References: <20140521015948.GA21600@brightrain.aerifal.cx> <20140522035942.GG507@brightrain.aerifal.cx> <537F92CA.8090808@redhat.com> <20140606171424.GC179@brightrain.aerifal.cx> <20140609184601.GI179@brightrain.aerifal.cx> <20140616090604.GB14894@kam.mff.cuni.cz> <20140616133829.GY179@brightrain.aerifal.cx> <20140616160519.GB12467@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140616160519.GB12467@kam.mff.cuni.cz> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-06/txt/msg01300.txt.bz2 On Mon, Jun 16, 2014 at 06:05:19PM +0200, Jan Hubicka wrote: > > > This needs your decl_replaceable change to not be optimized to if (0), > > > because of the explicit const modifier. > > > > The case I care about actually has "dummy" as const (with the intent > > that it be allocated in a read-only section if the dummy definition is > > used). So for me it's important that this regression be fixed too. > > Yep, GCC since 90's was optimizing reads from weak const attributes, but it > because worse because I added code walking through aliases. Ah, yes. BTW the reason I've avoided weak references and straight-out weak definitions in favor of using weak aliases (aside from aliases saving space in some cases) is that weak-alias seems to be the baseline feature that's "always worked safely" whereas the others have had bugs on and off in the past. And I really hope we can avoid having more than a single release where it's broken. Some distros are already pulling in 4.9.0 and I'm getting more bug reports. > > > I did not change ctor_for_folding to reject variables above as I was not quite > > > sure we want to support this kind of interposition and I am still not quite certain. > > > C++ is quite clear about the transformation replacing initialized const by its value. > > > > My concern is about C, not C++. This kind of interposition has always > > been supported in unix C, even prior to GCC, going back to sysv or > > earlier, as a documented feature (historically #pragma weak). It > > should not regress. If fixing it results in an regression with regards > > to optimizability of C++, perhaps this could be made > > language-specific, or (better) the C++ front-end could add an > > additional internal-use-only attribute to weak definitions it > > generates internally that permits constant-folding them, while not > > breaking the semantics for weak definitions provided by the user at > > the source level. > > Yes, I see your point and clearly we should not optimize with explicit weak attribute. > I wonder if decl_replaceable_p is however correct check here or if we want explicit check > for weak visibility. Isn't anything weak inherently replacable? It seems like using decl_replacable_p, if that predicate is correctly implemented, would be completely safe, since it should be true for a superset of weak. > I am concerned about const variables w/o weak attribute with -fPIC (because for > those decl_replaceable_p returns true, too). Consider following testcase: > struct t > { > static const int dummy=0; > const int *m(); > } t; > int > main() > { > return *t.m(); > } > int > main2() > { > return t.dummy; > } > const int * > t::m() > { > return &dummy; > } > > Here main2 is optimized by C++ FE to return 0, while backend is affraid to optimize > main() after inlining anticipating that dummy may be interposed. However moving t::m > inside of the unit will make dummy comdat and it will get optimizing. > Adding another method and keying the t into other unit will make it optimized, too. > > This is not very consistent. But perhaps we need a flag from C++ FE to tell us > what variables may not be interposed, because perhaps the c variant with -fPIC > const int dummy=0; > int > main() > { > return t; > } > > Jason? > > A C variant of the testcase: > > const int dummy=0; > > const static int * d=&dummy; > int > main() > { > return dummy; > } > int > main2() > { > return *d; > } > > seems optimized to return 0 (with -fPIC) for ages, too, but here at least > frontend won't substitute first dummy for 0. I see the issue of whether interposition should be supported in non-weak situations like this as completely separate from pr61144. Strictly speaking, it's undefined behavior to have multiple definitions of the same symbol. ELF semantics allow such interposition for shared library use mainly because creating a shared library hides translation-unit boundaries whereby symbol replacement with the equivalent static library may have had well-defined behavior due to the conflicting translation units not getting pulled in by the linker. But there's no fundamental reason to need to support interposition against symbols defined in the same translation unit, since in the corresponding static-library usage it would result in a link-time error. With the weak symbols issue (pr61144), on the other hand, the bug is that the well-established semantics of weak binding are not being honored by the compiler. Of course it may still be _desirable_ to support the sort of interposition you described for definitions in the same translation unit, but whether you want to support that should be treated as a separate issue IMO, for the above reasons. Rich