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

[Uros Bizjak <ubizjak@gmail.com>]

On Mon, Mar 18, 2024 at 3:51 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Thu, Mar 07, 2024 at 11:46:54PM +0100, Uros Bizjak wrote:
> > > Can't you just describe the dataflow then, without an unspec?  An unspec
> > > by definition does some (unspecified) operation on the data.
> >
> > Previously, it was defined as:
> >
> >  (define_insn "*pushfl<mode>2"
> >    [(set (match_operand:W 0 "push_operand" "=<")
> >  (match_operand:W 1 "flags_reg_operand"))]
> >
> > But Wmode, AKA SI/DImode is not CCmode. And as said in my last
> > message, nothing prevents current source code to try to update the CC
> > register here.
>
> So you can use an unspec just to convert the flags reg to an integer?
> To make an integer representation of flags reg contents.

Yes, this is correct. But please note the v3 patch, where the mode
update is made at the correct location. Quote from the patch:

Replace cc_use_loc with the entire new RTX only in case cc_use_loc satisfies
COMPARISON_P predicate.  Otherwise scan the entire cc_use_loc RTX for CC reg
to be updated with a new mode.

> Or is that what we started with here?

The reason for the patch is that when CC reg is used outside
comparison RTX, the combine tries to update CC reg mode where it is
used after the combined instruction. This happens on extremely rare
occasions, but when it happens, combine assumes that it is used
exclusively in the comparison RTX and uses "SUBST (XEXP (*cc_use_loc,
0), ...);". XEXP (*cc_use_loc, 0) will segfault when CC reg is
referred in a simple SET assignment, not only when it is referred in
an UNSPEC. Please note that the comparison RTX is handled a few lines
above, where my patch also fixes the "???" issue.

Uros.