From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25086 invoked by alias); 17 Apr 2015 11:34:20 -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 25064 invoked by uid 89); 17 Apr 2015 11:34:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f176.google.com Received: from mail-lb0-f176.google.com (HELO mail-lb0-f176.google.com) (209.85.217.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 17 Apr 2015 11:34:18 +0000 Received: by lbbuc2 with SMTP id uc2so80448841lbb.2 for ; Fri, 17 Apr 2015 04:34:14 -0700 (PDT) X-Received: by 10.112.35.230 with SMTP id l6mr2699786lbj.5.1429270454574; Fri, 17 Apr 2015 04:34:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.163.129 with HTTP; Fri, 17 Apr 2015 04:33:54 -0700 (PDT) In-Reply-To: <20150417074655.GA3186@atmel.com> References: <20150416064347.GA31491@atmel.com> <552F7A8D.1080709@gjlay.de> <20150416092802.GC31491@atmel.com> <552FE79E.2040509@gjlay.de> <20150417074655.GA3186@atmel.com> From: Denis Chertykov Date: Fri, 17 Apr 2015 11:34:00 -0000 Message-ID: Subject: Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call To: Senthil Kumar Selvaraj Cc: Georg-Johann Lay , gcc-patches Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-04/txt/msg00870.txt.bz2 2015-04-17 10:46 GMT+03:00 Senthil Kumar Selvaraj : > > On Thu, Apr 16, 2015 at 06:47:26PM +0200, Georg-Johann Lay wrote: > > Am 04/16/2015 um 11:28 AM schrieb Senthil Kumar Selvaraj: > > >On Thu, Apr 16, 2015 at 11:02:05AM +0200, Georg-Johann Lay wrote: > > >>Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj: > > >>>This patch fixes PR 65657. > > >> > > >>The following artifact appears to be PR63633. > > >> > > > > > >I did see that one - unfortunately, that fix won't help here. IIUC, you > > >check if input/output operand hard regs are in the clobber list, > > >and if yes, you generate pseudos to save and restore clobbered hard > > >regs. > > > > > >In this case, the reg is actually clobbered by a different insn (one > > > > Arrgh, yes... > > > > >that loads the next argument to the function). So unless I blindly generate pseudos for > > >all regs in the clobber list, the clobbering will still happen. > > > > > >FWIW, I did try saving/restoring all clobbered regs, and it did fix the > > >problem - just that it appeared like a (worse) hack to me. Aren't we > > >manually replicating what RA/reload should be doing? > > > > As it appears, we'll have to do it by hand. The attaches patch is just a > > sketch that indicates how the problem could be approached. Notice the new > > assertions in the split expanders; they will throw ICE until the fix is > > actually installed. > > > > The critical insn are generated in movMM expander and are changed to have no > > clobbers (SCRATCHes actually). An a later pass, when appropriate life info > > can be made available, run yet another avr pass that > > > > 1a) Save-restore needed hard regs around the insn. > > > > 2a) Kick out hard regs overlapping the clobbers, e.g. in set_src, into new > > pseudos. Maybe that could happen due to some hard regs progagation, or we > > can use a new predicate similar combine_pseudo_register_operand. > > > > 3) Replace scratch -> hard regs for all scratch_operands. > > > > 2b) Restore respective hard regs from their pseudos. > > > > 1b) Restore respective hard regs from their pseudos. > > > > > > And maybe we can get better code by allocating things like address register > > by hand and get better code then. > > > > When I implemented some of the libgcc insns I tried to express the operand > > by means of constraints, e.h. for (reg:HI 22) and let register allocator do > > the job. > > > > The resulting code was functional but *horrific*. > > > > The register allocator is not yet ready to generate efficient code in such > > demanding situations... > > > > > > > >What do you think? > > > > > > > IMO sooner or later we'll need such an infrastructure; maybe also for > > non-mov insn that are implemented by transparent libcalls like divmod, mul, > > etc. > > I'm curious how other embedded targets handle this though - arm, for > example? Surely they should have some libcalls/builtins which need > specific hard regs? I should check that out. > > > > > >>>When cfgexpand.c expands a function call, it first figures out the > > >>>registers in which the arguments will go, followed by expansion of the > > >>>arguments themselves (right to left). It later emits mov insns to set > > >>>the precomputed registers with the expanded RTXes. > > >>> > > >>>If one of the arguments is a __memx char pointer dereference, the mov > > >>>eventually expands to gen_xload_A (for certain cases), which > > >>>clobbers R22, R21 and Z. This causes problems if one of those > > >>>clobbered registers was precomputed to hold another argument. > > >>> > > >>>In general, call expansion does not appear to take clobbers into account - > > > > We had been warned that using hard regs is evil... But without that > > technique the code quality would decrease way too much. > > Oh ok - do you know some place where this is documented or was > discussed? https://gcc.gnu.org/ml/gcc/2014-10/msg00207.html