[PATCH, committed] SH: Fix PR58314 (unsatisfied constraints)

[Oleg Endo]

On Wed, 2013-09-18 at 09:55 +0200, Christian Bruel wrote:
> Hi Richard,
> 
> On 09/16/2013 07:10 PM, Richard Sandiford wrote:
> > Hi Christian,
> >
> > Christian Bruel <christian.bruel@st.com> writes:
> >> @@ -6893,11 +6894,14 @@ 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")
> >> -	(match_operand:QIHI 1 "register_operand" "r"))]
> >> +  [(set (match_operand:QIHI 0 "arith_reg_dest" "=r,m,*z")
> >> +	(match_operand:QIHI 1 "register_operand" "r,*z,m"))]
> > If the constraints allow "m", the predicates need to accept memories too.
> > (It'd be worth having an insn condition that rejects both operands
> > being memories though.)
> >
> > Thanks,
> > Richard
> Thanks for your comment,
> 
> I was wondering this too when doing the fix. I felt that a memory
> operand would be matched by the *movhi" patterns bellow.  As  I wanted
> to fix only the spilling case, so the original operand is a pseudo reg
> having matched the register predicate.
> Without the predicate memory not found, I wonder how I never hit a kind
> of "insn not found" error,  well, 'll give a try to adding a memory
> condition in the predicate, 
> but I fear that the movhi patterns will stop
> to match,

Yes, this will be the case.  The order of the movhi and movqi patterns
in the md file is important.  To address the predicates vs. constraints
issue, the following seems to work:

(define_insn "*mov<mode>_reg_reg"
  [(set (match_operand:QIHI 0 "general_movsrc_operand" "=r,m,*z")
	(match_operand:QIHI 1 "general_movdst_operand" "r,*z,m"))]
  "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode)
   && (arith_reg_operand (operands[0], <MODE>mode)
       || arith_reg_operand (operands[1], <MODE>mode))
   && (!can_create_pseudo_p () && REG_P (operands[0]) && REG_P (operands[1]))"
  "@
	mov	%1,%0
	mov.<bw>	%1,%0
	mov.<bw>	%1,%0"
  [(set_attr "type" "move,store,load")])

.. at least it survives the test case for this PR.  I haven't done
further tests.

BTW, in the test case (gcc.target/sh/torture/pr58314.c), this 

/* { dg-options "-Os" } */

defeats the purpose of the torture tests.

Cheers,
Oleg