| Message ID | 523AB982.9050303@st.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Thu, 2013-09-19 at 10:44 +0200, Christian Bruel wrote: > Hi Kaz, Oleg, > > On 09/19/2013 01:15 AM, Kaz Kojima wrote: > > Christian Bruel <christian.bruel@st.com> wrote: > >> && (!can_create_pseudo_p () && REG_P (operands[0]) && REG_P (operands[1]))" > >> > >> is necessary ? > > It looks an another hack to allow the 2nd and 3rd alternatives only > > when reloading. If so, it might be a bit cleaner to use a special > > predicate like > > > > > This still looks complicated to me. I have tested for sh-superh-elf and > sh-linux the attached patch that just "fixes" the issue reported by > Richard with no regression and absolutely no differences in code > generation for CSIBe and a few other benches (eembc, coremark, ...). > The spill alternatives are correctly selected and the original PR still > passes. > > If OK I'd like to apply it to trunk/4.8. If there is the need for an > additional hack, How about sending it separately ? Yeah, the move patterns probably could use some cleanup / refactoring anyway. I also wonder what is going to happen if LRA is used ... but that's another story. Have you also checked the patch for SH2A? Cheers, Oleg
Christian Bruel <christian.bruel@st.com> writes: > Index: gcc/config/sh/sh.md > =================================================================== > --- gcc/config/sh/sh.md (revision 202699) > +++ gcc/config/sh/sh.md (working copy) > @@ -6894,9 +6894,11 @@ label: > ;; reloading MAC subregs otherwise. For that probably special patterns > ;; would be required. > (define_insn "*mov<mode>_reg_reg" > - [(set (match_operand:QIHI 0 "arith_reg_dest" "=r,m,*z") > - (match_operand:QIHI 1 "register_operand" "r,*z,m"))] > - "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode)" > + [(set (match_operand:QIHI 0 "general_movdst_operand" "=r,m,*z") > + (match_operand:QIHI 1 "general_movsrc_operand" "r,*z,m"))] > + "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode) > + && arith_reg_dest (operands[0], <MODE>mode) > + && register_operand (operands[1], <MODE>mode)" This defeats the purpose of changing the predicates though. The problem with the original pattern was that you shouldn't have a situation where the constraints allow a combination that recog wouldn't match to the same define_insn. Constraints must always match a subset of what recog would match. Sorry for just saying something's wrong without suggesting a fix, but I don't know anything about the SH port. In general though, the "r<-r", "r<-m" and "m<-r" alternatives should be part of a single define_insn, rather than split across several. Which sounds like what Oleg was suggesting about folding the r<-r alternatives back into the main patterns. Thanks, Richard
2013-09-13 Christian Bruel <christian.bruel@st.com> * config/sh/sh.md (mov<mode>_reg_reg): Use general_movd*_operand predicate and guard insn with reg only operand. Index: gcc/config/sh/sh.md =================================================================== --- gcc/config/sh/sh.md (revision 202699) +++ gcc/config/sh/sh.md (working copy) @@ -6894,9 +6894,11 @@ label: ;; reloading MAC subregs otherwise. For that probably special patterns ;; would be required. (define_insn "*mov<mode>_reg_reg" - [(set (match_operand:QIHI 0 "arith_reg_dest" "=r,m,*z") - (match_operand:QIHI 1 "register_operand" "r,*z,m"))] - "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode)" + [(set (match_operand:QIHI 0 "general_movdst_operand" "=r,m,*z") + (match_operand:QIHI 1 "general_movsrc_operand" "r,*z,m"))] + "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode) + && arith_reg_dest (operands[0], <MODE>mode) + && register_operand (operands[1], <MODE>mode)" "@ mov %1,%0 mov.<bw> %1,%0