On Tue, Jun 8, 2021 at 12:05 AM Segher Boessenkool
<seg...@kernel.crashing.org> wrote:
>
> In theory we could have a split condition not inclusive of the insn
> condition in the past. That never was a good idea, the code does not do
> what a non-suspicious reader would think it does. But it leads to more
> serious problems together with iterators: if the split condition (as
> written) does not start with "&&", you do not get the insn condition
> included in the split condition, and that holds for the part of the insn
> condition that was generated by the iterator as well!
>
> This patch simply always joins the two conditions (after the iterators
> have done their work) to get the effective split condition.
>
> I tested this on all Linux targets, building the Linux kernel for each,
> and it does not change generated code for any of them, so I think we do
> not have much breakage to fear. But it is possible for other targets of
> course, and for floating point or vector code, etc.
>
> Is this okay for trunk?
Even if it looks uglier I would prefer to enforce a leading "&& " on the
split condition. That keeps the semantic of the define_insn_and_split
the same on trunk and branches and thus maintaining things easier.
I suppose once branches without such enforcement go out of
maintainance we can mass-strip the "&& "s.
I guess a mass-change to add "&& "s at this point is smaller than
a corresponding change to drop them (IMHO leaving both after this
change would be confusing).
Richard.
>
> Segher
>
>
> 2021-06-07 Segher Boessenkool <seg...@kernel.crashing.org>
>
> * gensupport.c (process_rtx) [DEFINE_INSN_AND_SPLIT]: Always include
> the insn condition in the split condition.
>
> ---
> gcc/gensupport.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/gensupport.c b/gcc/gensupport.c
> index 2cb760ffb90f..8a6345d36470 100644
> --- a/gcc/gensupport.c
> +++ b/gcc/gensupport.c
> @@ -590,7 +590,6 @@ process_rtx (rtx desc, file_location loc)
> case DEFINE_INSN_AND_SPLIT:
> case DEFINE_INSN_AND_REWRITE:
> {
> - const char *split_cond;
> rtx split;
> rtvec attr;
> int i;
> @@ -609,17 +608,25 @@ process_rtx (rtx desc, file_location loc)
> remove_constraints (XVECEXP (split, 0, i));
> }
>
> - /* If the split condition starts with "&&", append it to the
> - insn condition to create the new split condition. */
> - split_cond = XSTR (desc, 4);
> - if (split_cond[0] == '&' && split_cond[1] == '&')
> + const char *insn_cond = XSTR (desc, 2);
> + const char *split_cond = XSTR (desc, 4);
> + if (strncmp (split_cond, "&&", 2)
> + && GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> + error_at (loc, "the rewrite condition must start with `&&'");
> +
> + /* If the split condition starts with "&&", skip that. */
> + if (!strncmp (split_cond, "&&", 2))
> {
> rtx_reader_ptr->copy_md_ptr_loc (split_cond + 2, split_cond);
> - split_cond = rtx_reader_ptr->join_c_conditions (XSTR (desc, 2),
> - split_cond + 2);
> + split_cond += 2;
> }
> - else if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> - error_at (loc, "the rewrite condition must start with `&&'");
> +
> + /* Always use the conjunction of the given split condition and the
> + insn condition (which includes stuff from iterators, it is not just
> + what is given in the pattern in the machine description) as the
> + split condition to use. */
> + split_cond = rtx_reader_ptr->join_c_conditions (insn_cond,
> split_cond);
> +
> XSTR (split, 1) = split_cond;
> if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> XVEC (split, 2) = gen_rewrite_sequence (XVEC (desc, 1));
> --
> 1.8.3.1
>
