| Message ID | CADj25HNmQLe5HTW8Loa7vKJQpww5xypeZgr-YidRik29F0xZ=g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 25, 2013 at 10:04 AM, Chung-Ju Wu <jasonwucj@gmail.com> wrote: >>>>>> * config/alpha/alpha.c (TARGET_LRA_P): New define. >>>>>> >>>>>> Bootstrapped and regression tested [1] on alphaev68-unknown-linux-gnu. >>>>>> >>>>>> OK for 4.9? >>>>>> >>>>> >>>>> Yep. >>>> >>>> >>>> Unfortunately, alphas are much more tied to reload than it was hoped. >>>> While latest alphas (with FIX and BWX ISAs) survived transition to LRA >>>> without problems, further testing on ev4 and ev5 triggered various >>>> problems, one of them is PR57032 [1] that exposed rather unique way of >>>> handling aligned/nonaligned memory operands. >>>> >>>> The patch was reverted. >>>> >>>> I suspect that fixing older alphas to live with LRA would be quite >>>> involved task, and I guess nobody (including me) wants to spend >>>> considerable amount of time on a dying architecture. Consequently, >>>> this also means that alphas will die together with reload as far as >>>> gcc is concerned. >>>> >>>> [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57032 >>> >>> Would it make sense to deprecate the older Alpha implementations without >>> killing the "modern" ones? >> >> Right. That would also eliminate the NOTE_INSN_EH_REGION notes bug (PR >> target/56858). >> >> I think it would be a shame to not enable LRA on alpha. It will only >> be another excuse to never let reload die, and it will hurt stability >> and reliability for Alpha EV6 in the long term as other targets switch >> over to LRA. >> >> Is it possible to add some EV4/EV5 splitters to work around this Alpha >> EV4/EV5 oddity? Even if it comes at a code quality cost, it's IMHO >> still better than tying the fate of apha to reload and vice versa.. >> >> Ciao! >> Steven > > > How about using follow constraints? > > Index: alpha.c > =================================================================== > --- alpha.c (revision 198216) > +++ alpha.c (working copy) > @@ -9871,6 +9871,9 @@ > #undef TARGET_LEGITIMATE_ADDRESS_P > #define TARGET_LEGITIMATE_ADDRESS_P alpha_legitimate_address_p > > +#undef TARGET_LRA_P > +#define TARGET_LRA_P hook_bool_void_true > + > #undef TARGET_CONDITIONAL_REGISTER_USAGE > #define TARGET_CONDITIONAL_REGISTER_USAGE alpha_conditional_register_usage > > Index: alpha.md > =================================================================== > --- alpha.md (revision 198216) > +++ alpha.md (working copy) > @@ -4073,9 +4073,9 @@ > > (define_insn "*movdi" > [(set (match_operand:DI 0 "nonimmediate_operand" > - "=r,r,r,r,r,r,r,r, m, *f,*f, Q, r,*f") > + "=r,r,r,r,r,r,r,r, m, *f,*f, m, r,*f") > (match_operand:DI 1 "input_operand" > - "rJ,K,L,T,s,n,s,m,rJ,*fJ, Q,*f,*f, r"))] > + "rJ,K,L,T,s,n,s,m,rJ,*fJ, m,*f,*f, r"))] > "register_operand (operands[0], DImode) > || reg_or_0_operand (operands[1], DImode)" > "@ > > > As Uros said, the 'Q' is ignored by LRA. > The reason is that the predicate function normal_memory_operand() > may change op to a memory location by using resolved_reload_operand(). > However, if LRA is enabled, resolve_reload_operand() always returns > original reg op because reload_in_progress would never be true, > resulting reload loop in this case. > > So I guess using 'm' constraint instead of 'Q' is able to avoid > such abnormal behavior, leaving all the reload jobs to LRA. > IMHO that is a simplest solution. At least it passes the case in > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57032 > and successfully build libgcc. Yes, this patch will work up to building libstdc++, where it will fail with the same reason on movqi pattern (on non-BWX target). I suspect that QImode access is generated during LRA (where reload_in_progress is false!) and it directly generates movqi, which can't use memory operand. Maybe simply adding lra_in_progress to reload_in_progress would fix this problem. Uros.
On 2013-04-25 09:18, Uros Bizjak wrote: > Yes, this patch will work up to building libstdc++, where it will fail > with the same reason on movqi pattern (on non-BWX target). I suspect > that QImode access is generated during LRA (where reload_in_progress > is false!) and it directly generates movqi, which can't use memory > operand. Maybe simply adding lra_in_progress to reload_in_progress > would fix this problem. Possibly. I've wondered in the past if it wouldn't be better to spill QI/HImode values in SImode for pre-BWX targets. Back in the day that would have involved hacking reload, but now it would appear that SECONDARY_MEMORY_NEEDED_RTX plus a reload_[qh]i pattern would do the job. Of course, that macro itself is in reload.c... Is there some other way we're supposed to get the same effect from LRA? r~
Index: alpha.c =================================================================== --- alpha.c (revision 198216) +++ alpha.c (working copy) @@ -9871,6 +9871,9 @@ #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P alpha_legitimate_address_p +#undef TARGET_LRA_P +#define TARGET_LRA_P hook_bool_void_true + #undef TARGET_CONDITIONAL_REGISTER_USAGE #define TARGET_CONDITIONAL_REGISTER_USAGE alpha_conditional_register_usage Index: alpha.md =================================================================== --- alpha.md (revision 198216) +++ alpha.md (working copy) @@ -4073,9 +4073,9 @@ (define_insn "*movdi" [(set (match_operand:DI 0 "nonimmediate_operand" - "=r,r,r,r,r,r,r,r, m, *f,*f, Q, r,*f") + "=r,r,r,r,r,r,r,r, m, *f,*f, m, r,*f") (match_operand:DI 1 "input_operand" - "rJ,K,L,T,s,n,s,m,rJ,*fJ, Q,*f,*f, r"))] + "rJ,K,L,T,s,n,s,m,rJ,*fJ, m,*f,*f, r"))] "register_operand (operands[0], DImode) || reg_or_0_operand (operands[1], DImode)" "@