| Message ID | 20121107.173940.2087216866847625200.davem@davemloft.net |
|---|---|
| State | New |
| Headers | show |
On 12-11-07 5:39 PM, David Miller wrote: > Vlad, I wanted to make you aware of the following because it's > a major barrier for using LRA on sparc at this time. I therefore > do not think moving to LRA on this target is possible in the 4.8 > timeframe, which is fine. The situation is described completely > in the comment I am adding in the patch below. David, thanks very much for reporting this. I was quite surprised when you decided to switch SPARC to LRA for gcc4.8. I was a bit scary too because I never paid a big attention to SPARC. Even LRA for x86/x86-64 on which I worked hard is not easy to port and I have a lot of PRs now. I did eight LRA ports on the branch mostly to prove that LRA approach is doable to replace reload. So your decision is a big relief for me. Thanks very much for working on SPARC LRA port. I am sure we solve all the problems and switch on LRA for SPARC for gcc4.9. I'll return my work on other targets and SPARC on the branch when I am done with current LRA PRs for x86/x86-64. > The most alarming aspect of this to me was discovering that IRA could > allocate registers to a pseudo that did not pass HARD_REGNO_MODE_OK, > and this anomaly is completely masked because reload and our splitters > end up fixing things up. I think SPARC port is far from the worst. I also found recently some inconsistency in LRA and IRA (LRA recognizes conflicts for multi-word pseudos when IRA does not) which I'd like to address too. I think Richard Sandiford is probably right saying that we should use LRA as opportunity to clean the ports. > I wanted to explicitly thank you for your work on LRA because without > it we would never have discovered these inconsistencies in the sparc > backend. > >
On Wed, Nov 7, 2012 at 11:39 PM, David Miller wrote: > One idea that occurred to me was perhaps to extend > define_register_constraint such that an extra condition can be > specified. So for sparc's constraint "U" it would evaluate to > GENERAL_REGS but also express the condition that the hard register > must be even. I haven't looked at the details of this all, but there are ports that use define_predicate to request an even-numbered register. See e.g. frv and v850. I'm not sure if/how the RA takes predicates into account when selecting a register. (bfin uses define_register_constraints, but it has separate register classes for the even-numbered registers, so apparently that's not for multi-word hardregs like your case.) > diff --git a/gcc/config/sparc/constraints.md b/gcc/config/sparc/constraints.md > index 2f8c6ad..440dc57 100644 > --- a/gcc/config/sparc/constraints.md > +++ b/gcc/config/sparc/constraints.md > @@ -130,7 +130,43 @@ > (match_code "mem") > (match_test "memory_ok_for_ldd (op)"))) > > -;; Not needed in 64-bit mode > +;; This awkward register constraint is necessary because it is not > +;; possible to express the "must be even numbered regsiter" condition s/regsiter/register/ Ciao! Steven
From: Steven Bosscher <stevenb.gcc@gmail.com> Date: Thu, 8 Nov 2012 01:19:11 +0100 > On Wed, Nov 7, 2012 at 11:39 PM, David Miller wrote: >> One idea that occurred to me was perhaps to extend >> define_register_constraint such that an extra condition can be >> specified. So for sparc's constraint "U" it would evaluate to >> GENERAL_REGS but also express the condition that the hard register >> must be even. > > I haven't looked at the details of this all, but there are ports that > use define_predicate to request an even-numbered register. See e.g. > frv and v850. I'm not sure if/how the RA takes predicates into account > when selecting a register. That would only influence instruction recognition. > (bfin uses define_register_constraints, but it has separate register > classes for the even-numbered registers, so apparently that's not for > multi-word hardregs like your case.) Right. >> diff --git a/gcc/config/sparc/constraints.md b/gcc/config/sparc/constraints.md >> index 2f8c6ad..440dc57 100644 >> --- a/gcc/config/sparc/constraints.md >> +++ b/gcc/config/sparc/constraints.md >> @@ -130,7 +130,43 @@ >> (match_code "mem") >> (match_test "memory_ok_for_ldd (op)"))) >> >> -;; Not needed in 64-bit mode >> +;; This awkward register constraint is necessary because it is not >> +;; possible to express the "must be even numbered regsiter" condition > > s/regsiter/register/ Thanks, I'll fix that.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 24d9845..64e7596 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2012-11-07 David S. Miller <davem@davemloft.net> + + * config/sparc/constraints.md ("U"): Document, in detail, + which this constraint is necessary. + 2012-11-07 Richard Henderson <rth@redhat.com> * trans-mem.c (pass_ipa_tm): Don't use TODO_update_ssa. diff --git a/gcc/config/sparc/constraints.md b/gcc/config/sparc/constraints.md index 2f8c6ad..440dc57 100644 --- a/gcc/config/sparc/constraints.md +++ b/gcc/config/sparc/constraints.md @@ -130,7 +130,43 @@ (match_code "mem") (match_test "memory_ok_for_ldd (op)"))) -;; Not needed in 64-bit mode +;; This awkward register constraint is necessary because it is not +;; possible to express the "must be even numbered regsiter" condition +;; using register classes. The problem is that membership in a +;; register class requires that all registers of a multi-regno +;; register be included in the set. It is add_to_hard_reg_set +;; and in_hard_reg_set_p which populate and test regsets with these +;; semantics. +;; +;; So this means that we would have to put both the even and odd +;; register into the register class, which would not restrict things +;; at all. +;; +;; Using a combination of GENERAL_REGS and HARD_REGNO_MODE_OK is not a +;; full solution either. In fact, even though IRA uses the macro +;; HARD_REGNO_MODE_OK to calculate which registers are prohibited from +;; use in certain modes, it still can allocate an odd hard register +;; for DImode values. This is due to how IRA populates the table +;; ira_useful_class_mode_regs[][]. It suffers from the same problem +;; as using a register class to describe this restriction. Namely, it +;; sets both the odd and even part of an even register pair in the +;; regset. Therefore IRA can and will allocate odd registers for +;; DImode values on 32-bit. +;; +;; There are legitimate cases where DImode values can end up in odd +;; hard registers, the most notable example is argument passing. +;; +;; What saves us is reload and the DImode splitters. Both are +;; necessary. The odd register splitters cannot match if, for +;; example, we have a non-offsetable MEM. Reload will notice this +;; case and reload the address into a single hard register. +;; +;; The real downfall of this awkward register constraint is that it does +;; not evaluate to a true register class like a bonafide use of +;; define_register_constraint would. This currently means that we cannot +;; use LRA on Sparc, since the constraint processing of LRA really depends +;; upon whether an extra constraint is for registers or not. It uses +;; REG_CLASS_FROM_CONSTRAINT, and checks it against NO_REGS. (define_constraint "U" "Pseudo-register or hard even-numbered integer register" (and (match_test "TARGET_ARCH32")
Vlad, I wanted to make you aware of the following because it's a major barrier for using LRA on sparc at this time. I therefore do not think moving to LRA on this target is possible in the 4.8 timeframe, which is fine. The situation is described completely in the comment I am adding in the patch below. The most alarming aspect of this to me was discovering that IRA could allocate registers to a pseudo that did not pass HARD_REGNO_MODE_OK, and this anomaly is completely masked because reload and our splitters end up fixing things up. I wanted to explicitly thank you for your work on LRA because without it we would never have discovered these inconsistencies in the sparc backend. One idea that occurred to me was perhaps to extend define_register_constraint such that an extra condition can be specified. So for sparc's constraint "U" it would evaluate to GENERAL_REGS but also express the condition that the hard register must be even. Then we could make the implementation of the macro REG_CLASS_FROM_CONSTRAINT test the extra condition specified in define_register_constraint, and return NO_REGS if that condition does not pass. But it would be much nicer if register classes could do what we need them to. Such a solution would be both cleaner, and significantly more efficient. * config/sparc/constraints.md ("U"): Document, in detail, which this constraint is necessary. --- gcc/ChangeLog | 5 +++++ gcc/config/sparc/constraints.md | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-)