Re: [PATCH] combine: Fix ICE in try_combine on pr112494.c [PR112560]

[Uros Bizjak <ubizjak@gmail.com>]

On Thu, Nov 30, 2023 at 9:21 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Wed, Nov 29, 2023 at 02:20:03PM +0100, Uros Bizjak wrote:
> > On Wed, Nov 29, 2023 at 1:25 PM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > > On Wed, Nov 29, 2023 at 10:35 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > I was assuming that if the CC reg is not used inside the comparison,
> > then the mode of CC reg is irrelevant. We can still combine the
> > instructions into new insn, without updating the use of CC reg.
>
> It should never happen that the CC reg is not used, so what does this
> mean?

I was trying to say that when CC reg is used inside the comparison, e.g.:

(define_insn "*setcc_qi"
  [(set (match_operand:QI 0 "nonimmediate_operand" "=qm")
    (match_operator:QI 1 "ix86_comparison_operator"
      [(reg FLAGS_REG) (const_int 0)]))]
  ""
  "set%C1\t%0"

and through the%C operand modifier we call put_condition_code, the
mode of CC reg is checked and a different instruction is emitted,
depending on the CC reg mode. Please also note that not all comparison
operators are handled, so when the combine rewrites the comparison, it
isn't necessarily supported.

This is in contrast with:

(define_insn "@pushfl<mode>2"
  [(set (match_operand:W 0 "push_operand" "=<")
    (unspec:W [(match_operand:CC 1 "flags_reg_operand")]
          UNSPEC_PUSHFL))]

where we don't care about CC reg mode at all. The CC reg is not in the
comparison, so the mode does not matter. The combination of
instructions should not be limited by CC reg user in this case.

>
> Where it is used might not be a comparison of course, as in your
> example.
>
> >         && (cc_use_loc = find_single_use (SET_DEST (newpat), i3,
> >                                           &cc_use_insn)))
> >       {
> > -       compare_code = orig_compare_code = GET_CODE (*cc_use_loc);
> > -       if (is_a <scalar_int_mode> (GET_MODE (i2dest), &mode))
> > -         compare_code = simplify_compare_const (compare_code, mode,
> > -                                                &op0, &op1);
> > -       target_canonicalize_comparison (&compare_code, &op0, &op1, 1);
> > +       if (COMPARISON_P (*cc_use_loc))
> > +         {
> > +           compare_code = orig_compare_code = GET_CODE (*cc_use_loc);
> > +           if (is_a <scalar_int_mode> (GET_MODE (i2dest), &mode))
> > +             compare_code = simplify_compare_const (compare_code, mode,
> > +                                                    &op0, &op1);
> > +           target_canonicalize_comparison (&compare_code, &op0, &op1, 1);
> > +         }
> > +       else
> > +         {
> > +           if (dump_file && (dump_flags & TDF_DETAILS))
> > +             fprintf (dump_file, "CC register not used in comparison.\n");
>
> "Where the CCmode register is used is not a comparison".  But more
> compact if you can manage (I cannot).

Perhaps "CCmode register user is not a comparison." ?

Uros.