[04/11] cris: Update unexpected empty split condition

gcc/ChangeLog:

	* config/cris/cris.md (*addi_reload): Fix empty split condition.
---
 gcc/config/cris/cris.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

> From: Kewen Lin <linkw@linux.ibm.com>
> Date: Wed, 2 Jun 2021 07:04:54 +0200

> gcc/ChangeLog:
> 
> 	* config/cris/cris.md (*addi_reload): Fix empty split condition.
> ---
>  gcc/config/cris/cris.md | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/config/cris/cris.md b/gcc/config/cris/cris.md
> index 7de0ec63fcf..d5a3c703a83 100644
> --- a/gcc/config/cris/cris.md
> +++ b/gcc/config/cris/cris.md
> @@ -1311,7 +1311,7 @@ (define_insn_and_split "*addi_reload"
>     && (INTVAL (operands[3]) == 2 || INTVAL (operands[3]) == 4)
>     && (reload_in_progress || reload_completed)"
>    "#"
> -  ""
> +  "&& 1"
>    [(set (match_dup 0)
>  	(plus:SI (ashift:SI (match_dup 2) (match_dup 3)) (match_dup 1)))]
>    "operands[3] = operands[3] == const2_rtx ? const1_rtx : const2_rtx;")
> -- 
> 2.17.1
> 

Ok, thanks, if only for all-round consistency.

In preparation for a warning for an empty condition?  I'm
usually all for .md-warnings, but I'm not sure about the
benefit of that one, though.  Those "&& 1" look...hackish.

brgds, H-P

Hi Nilsson,

on 2021/6/2 下午8:45, Hans-Peter Nilsson wrote:
>> From: Kewen Lin <linkw@linux.ibm.com>
>> Date: Wed, 2 Jun 2021 07:04:54 +0200
> 
>> gcc/ChangeLog:
>>
>> 	* config/cris/cris.md (*addi_reload): Fix empty split condition.
>> ---
>>  gcc/config/cris/cris.md | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/config/cris/cris.md b/gcc/config/cris/cris.md
>> index 7de0ec63fcf..d5a3c703a83 100644
>> --- a/gcc/config/cris/cris.md
>> +++ b/gcc/config/cris/cris.md
>> @@ -1311,7 +1311,7 @@ (define_insn_and_split "*addi_reload"
>>     && (INTVAL (operands[3]) == 2 || INTVAL (operands[3]) == 4)
>>     && (reload_in_progress || reload_completed)"
>>    "#"
>> -  ""
>> +  "&& 1"
>>    [(set (match_dup 0)
>>  	(plus:SI (ashift:SI (match_dup 2) (match_dup 3)) (match_dup 1)))]
>>    "operands[3] = operands[3] == const2_rtx ? const1_rtx : const2_rtx;")
>> -- 
>> 2.17.1
>>
> 
> Ok, thanks, if only for all-round consistency.
> 
> In preparation for a warning for an empty condition?  I'm
> usually all for .md-warnings, but I'm not sure about the
> benefit of that one, though.  Those "&& 1" look...hackish.

Thanks!  Yeah, the 01/11 patch aims to raise one error message
for the define_insn_and_split whose split condition is empty
while insn condition isn't.  In most cases, when we write one
define_insn_and_split we want the splitting only to take effect
while we see the define_insn matching happen (insn cond holds),
but if we leave the split condition empty, the splitting will
be done always, it could result in some unexpected consequence.
Mostly this is unintentional.  The error message is to avoid
people to make it unintentionally.

As you may have seen from the discussion under the 00/11 thread,
we will probably end up with some other solution, so I will hold
the changes for the ports, sorry for wasting your time and the
other port maintainers'.

BR,
Kewen

> From: Kewen.Lin <linkw@linux.ibm.com>
> Date: Thu, 3 Jun 2021 07:45:57 +0200

> on 2021/6/2 Hans-Peter Nilsson wrote:
> >> From: Kewen Lin <linkw@linux.ibm.com>
> >> Date: Wed, 2 Jun 2021 07:04:54 +0200
> > 
> >> gcc/ChangeLog:
> >>
> >> 	* config/cris/cris.md (*addi_reload): Fix empty split condition.

> >> -  ""
> >> +  "&& 1"

> > Ok, thanks, if only for all-round consistency.
> > 
> > In preparation for a warning for an empty condition?  I'm
> > usually all for .md-warnings, but I'm not sure about the
> > benefit of that one, though.  Those "&& 1" look...hackish.
> 
> Thanks!  Yeah, the 01/11 patch aims to raise one error message
> for the define_insn_and_split whose split condition is empty
> while insn condition isn't.  In most cases, when we write one
> define_insn_and_split we want the splitting only to take effect
> while we see the define_insn matching happen (insn cond holds),
> but if we leave the split condition empty, the splitting will
> be done always, it could result in some unexpected consequence.
> Mostly this is unintentional.

It certainly was in the patch above!

>  The error message is to avoid
> people to make it unintentionally.
> 
> As you may have seen from the discussion under the 00/11 thread,
> we will probably end up with some other solution, so I will hold
> the changes for the ports, sorry for wasting your time and the
> other port maintainers'.

No worries: I certainly don't consider it wasted and I'd
prefer to have the patch above committed sooner than the
conclusion of that discussion.  (If you don't get to it,
I'll do it, after a round of testing.)

If you're considering further target patches to adjust for
eventually changed semantics in the define_insn_and_split
split-condition, then whatever trivial patch to cris.md that
gets the effect of the one you sent is preapproved.

Again, thanks.

brgds, H-P

> From: Hans-Peter Nilsson <hp@axis.com>
> CC: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
> Date: Thu, 3 Jun 2021 18:12:25 +0200

> I'd
> prefer to have the patch above committed sooner than the
> conclusion of that discussion.  (If you don't get to it,
> I'll do it, after a round of testing.)

Done; no regressions.

brgds, H-P

on 2021/6/4 上午12:12, Hans-Peter Nilsson wrote:
>> From: Kewen.Lin <linkw@linux.ibm.com>
>> Date: Thu, 3 Jun 2021 07:45:57 +0200
> 
>> on 2021/6/2 Hans-Peter Nilsson wrote:
>>>> From: Kewen Lin <linkw@linux.ibm.com>
>>>> Date: Wed, 2 Jun 2021 07:04:54 +0200
>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 	* config/cris/cris.md (*addi_reload): Fix empty split condition.
> 
>>>> -  ""
>>>> +  "&& 1"
> 
>>> Ok, thanks, if only for all-round consistency.
>>>
>>> In preparation for a warning for an empty condition?  I'm
>>> usually all for .md-warnings, but I'm not sure about the
>>> benefit of that one, though.  Those "&& 1" look...hackish.
>>
>> Thanks!  Yeah, the 01/11 patch aims to raise one error message
>> for the define_insn_and_split whose split condition is empty
>> while insn condition isn't.  In most cases, when we write one
>> define_insn_and_split we want the splitting only to take effect
>> while we see the define_insn matching happen (insn cond holds),
>> but if we leave the split condition empty, the splitting will
>> be done always, it could result in some unexpected consequence.
>> Mostly this is unintentional.
> 
> It certainly was in the patch above!
> 
>>  The error message is to avoid
>> people to make it unintentionally.
>>
>> As you may have seen from the discussion under the 00/11 thread,
>> we will probably end up with some other solution, so I will hold
>> the changes for the ports, sorry for wasting your time and the
>> other port maintainers'.
> 
> No worries: I certainly don't consider it wasted and I'd
> prefer to have the patch above committed sooner than the
> conclusion of that discussion.  (If you don't get to it,
> I'll do it, after a round of testing.)
> 

Thanks for your help on testing!

> If you're considering further target patches to adjust for
> eventually changed semantics in the define_insn_and_split
> split-condition, then whatever trivial patch to cris.md that
> gets the effect of the one you sent is preapproved.
> 

OK, thanks again!

BR,
Kewen