This is the mail archive of the
mailing list for the GCC project.
Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
- From: Denis Chertykov <chertykov at gmail dot com>
- To: Senthil Kumar Selvaraj <senthil_kumar dot selvaraj at atmel dot com>
- Cc: Georg-Johann Lay <avr at gjlay dot de>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 17 Apr 2015 14:33:54 +0300
- Subject: Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
- Authentication-results: sourceware.org; auth=none
- References: <20150416064347 dot GA31491 at atmel dot com> <552F7A8D dot 1080709 at gjlay dot de> <20150416092802 dot GC31491 at atmel dot com> <552FE79E dot 2040509 at gjlay dot de> <20150417074655 dot GA3186 at atmel dot com>
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<mode>_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