From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30161 invoked by alias); 28 Mar 2016 11:18:25 -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 30147 invoked by uid 89); 28 Mar 2016 11:18:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_20,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=200000, circumstances, handed, barriers X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 28 Mar 2016 11:18:22 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id u2SBIDLn010369; Mon, 28 Mar 2016 06:18:14 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id u2SBIDuu010368; Mon, 28 Mar 2016 06:18:13 -0500 Date: Mon, 28 Mar 2016 12:44:00 -0000 From: Segher Boessenkool To: Olivier Hainque Cc: GCC Patches Subject: Re: rs6000 stack_tie mishap again Message-ID: <20160328111812.GA17381@gate.crashing.org> References: <20160328030113.GB28596@gate.crashing.org> <4433372E-2AD5-4BF0-89E8-8739E64DD7B7@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4433372E-2AD5-4BF0-89E8-8739E64DD7B7@adacore.com> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg01459.txt.bz2 On Mon, Mar 28, 2016 at 12:45:03PM +0200, Olivier Hainque wrote: > > The normal -m32 compiler here generates code like > > > > lwz 11,0(1) > > > > and try as I might I cannot get it to fail. Maybe because the GPR11 > > setup here involves a load? > > You need to have had r11 last used to designate a global > symbol as part of the function body (in order to have base_term > designate a symbol_ref etc), and then have the scheduler > decide that moving across is a good idea. It's certainly not > an easy combination to trigger. Yes, I did that (with some asm's). Like this: === void g(int, char *); int dum; void f(int x) { char big[200000]; g(x, big); g(x, big); register void *p asm("r11") = &dum; asm("" : : "r"(p)); } === and the end of that becomes === bl g # 11 *call_nonlocal_sysvsi/1 [length = 4] lis 11,dum@ha # 12 elf_high [length = 4] la 11,dum@l(11) # 13 elf_low [length = 4] lwz 11,0(1) # 33 *movsi_internal1/3 [length = 4] lwz 0,4(11) # 34 *movsi_internal1/3 [length = 4] lwz 31,-4(11) # 36 *movsi_internal1/3 [length = 4] mr 1,11 # 38 *movsi_internal1/1 [length = 4] mtlr 0 # 35 *movsi_internal1/9 [length = 4] blr # 39 *return_internal_si [length = 4] === > > It seems clear that just considering the > > prologue is enough to fix the original problem (frame pointer setup), > > and problems like yours cannot happen in the prologue. > > Right. What is unclear is if it's correct to consider only > prologues here. ISTM we arrange to produce CFI for epilogues > as well today, at least in some circumstances, and maybe the > issue Richard had with prologues at the time would need to > be addressed for epilogues as well today. Correct is to do alias analysis in both the prologue and epilogue. If we don't, ports have to do all kinds of stack ties etc. to fake it. Currently we do neither, so doing just one is a step in the right direction. > > Better would be not to have this hack at all. > > Sure. > > >> My rough understanding is that we probably really care about frame_related > >> insns only here, at least on targets where the flag is supposed to be set > >> accurately. > > > > On targets with DWARF2 unwind info the flag should be set on those insns > > that need unwind info. This does not include all insns in the epilogue > > that access the frame, so I don't think this will help you? > > My idea was that, maybe, the insns we need to see for alias analysis > are precisely those for which the bit should not be set, which happened > to be exactly the case in the problematic situation we hit. But as I said, > I'm not 100% convinced the reasoning is globally correct. All the register restores do not have the flag set, in most cases. But they can in others (say, a target that does not optimise the CFI stuff very well). > >> This is the basis of the proposed patch, which essentially disconnects the > >> shortcut above for !frame_related insns on targets which need precise alias > >> analysis within epilogues, as indicated by a new target hook. > > > > Eww. Isn't that really all targets that schedule the epilogue at all? > > Yes. Most of them use stronger barriers which the dependency analyser > knows about, so aren't affected by this. > > That's a possible alternative approach for rs6000. A clobber of scratch should work yes. It's really heavy handed though, we can move the GPR1 restore quite freely otherwise (in shrink-wrap), but perhaps allowing scheduling to move it doesn't buy us much at all. This doesn't solve the problem however that other dependencies in the epilogue can be messed up in similar ways. Segher