[SH4] Fix PR58475 insn swapb does not satisfy its constraints

Hello,

This patch fixes the aforementioned PR by refusing FPUL_REG to be an
acceptable reg for any arithmetic_operand on TARGET_SH4. (This was a
strange SH4 singularity with regards to the SH family).

The only impacted insn is movsf_ie used for reg-fpreg transfers. So the
condition now mentions explicitly fpul_operand, allowing to simplify a
bit the logic to match by removing the extra checks.

The testsuite survived (no regression) for 
-m2,-m2a,-m2a-nofpu,-m2a-single,-m2a-single-only,-m3,-m3e,-m4,-m4-single,-m4-single-only,-m4a,-m4a-single,-m4a-single-only

No performance impact on a large number of benchmarks (CSIBE, EEMBC,
Coremark, ...)

sh4-linux-elf survived a full Linux distribution rebuild

OK for trunk?

many thanks,

Christian

Christian Bruel <christian.bruel@st.com> wrote:
> This patch fixes the aforementioned PR by refusing FPUL_REG to be an
> acceptable reg for any arithmetic_operand on TARGET_SH4. (This was a
> strange SH4 singularity with regards to the SH family).
> 
> The only impacted insn is movsf_ie used for reg-fpreg transfers. So the
> condition now mentions explicitly fpul_operand, allowing to simplify a
> bit the logic to match by removing the extra checks.
> 
> The testsuite survived (no regression) for 
> -m2,-m2a,-m2a-nofpu,-m2a-single,-m2a-single-only,-m3,-m3e,-m4,-m4-single,-m4-single-only,-m4a,-m4a-single,-m4a-single-only
> 
> No performance impact on a large number of benchmarks (CSIBE, EEMBC,
> Coremark, ...)
> 
> sh4-linux-elf survived a full Linux distribution rebuild
> 
> OK for trunk?

OK.

Regards,
	kaz

Christian Bruel <christian.bruel@st.com> wrote:
>  (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)"

I thought that predicates explicitly allowing mem only when reload
in progress are defensive because I guess there is no guarantee
that the condition part of the insn will be never used in spilling.
Re-factoring suggested by Oleg and Rechard would be the right thing
to do, though it might be a bit invasive for 4.8.

Regards,
	kaz

> Christian Bruel <christian.bruel@st.com> wrote:
>>  (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)"
> 
> I thought that predicates explicitly allowing mem only when reload
> in progress are defensive because I guess there is no guarantee
> that the condition part of the insn will be never used in spilling.
> Re-factoring suggested by Oleg and Rechard would be the right thing
> to do, though it might be a bit invasive for 4.8.

Ugh, this should be for

Re: [PATCH, committed] SH: Fix PR58314 (unsatisfied constraints)
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01447.html

Sorry for wrong reply.

Regards,
	kaz

On 09/20/2013 01:07 AM, Kaz Kojima wrote:
> Christian Bruel <christian.bruel@st.com> wrote:
>> This patch fixes the aforementioned PR by refusing FPUL_REG to be an
>> acceptable reg for any arithmetic_operand on TARGET_SH4. (This was a
>> strange SH4 singularity with regards to the SH family).
>>
>> The only impacted insn is movsf_ie used for reg-fpreg transfers. So the
>> condition now mentions explicitly fpul_operand, allowing to simplify a
>> bit the logic to match by removing the extra checks.
>>
>> The testsuite survived (no regression) for 
>> -m2,-m2a,-m2a-nofpu,-m2a-single,-m2a-single-only,-m3,-m3e,-m4,-m4-single,-m4-single-only,-m4a,-m4a-single,-m4a-single-only
>>
>> No performance impact on a large number of benchmarks (CSIBE, EEMBC,
>> Coremark, ...)
>>
>> sh4-linux-elf survived a full Linux distribution rebuild
>>
>> OK for trunk?
> OK.
>
> Regards,
> 	kaz

committed. Note that this fixes the ICE:

testsuite/gcc.c-torture/compile/pr55921.c:21:1: error: insn does not
satisfy its constraints:
(insn 128 33 129 (parallel [
            (set (reg:SF 65 fr1 [ cf ])
                (reg:SF 3 r3))
            (use (reg/v:PSI 151 ))
            (clobber (scratch:SI))
        ])

for -m2a (but still fails with  error: 'asm' operand requires impossible
reload  as previously with the other configurations)

Thanks

Christian