machmode: Introduce GET_MODE_NEXT_MODE with previous GET_MODE_WIDER_MODE meaning...

machmode: Introduce GET_MODE_NEXT_MODE with previous GET_MODE_WIDER_MODE meaning, add new GET_MODE_WIDER_MODE

On Wed, Oct 05, 2022 at 04:02:25PM -0400, Jason Merrill wrote:
> > > > @@ -5716,7 +5716,13 @@ emit_store_flag_1 (rtx target, enum rtx_
> > > >        {
> > > >         machine_mode optab_mode = mclass == MODE_CC ? CCmode : compare_mode;
> > > >         icode = optab_handler (cstore_optab, optab_mode);
> > > > -     if (icode != CODE_FOR_nothing)
> > > > +     if (icode != CODE_FOR_nothing
> > > > +        /* Don't consider [BH]Fmode as usable wider mode, as neither is
> > > > +           a subset or superset of the other.  */
> > > > +        && (compare_mode == mode
> > > > +            || !SCALAR_FLOAT_MODE_P (compare_mode)
> > > > +            || maybe_ne (GET_MODE_PRECISION (compare_mode),
> > > > +                         GET_MODE_PRECISION (mode))))
> > >
> > > Why do you need to do this here (and in prepare_cmp_insn, and similarly in
> > > can_compare_p)?  Shouldn't get_wider skip over modes that are not actually
> > > wider?
> >
> > I'm afraid too many places rely on all modes of a certain class to be
> > visible when walking from "narrowest" to "widest" mode, say
> > FOR_EACH_MODE_IN_CLASS/FOR_EACH_MODE/FOR_EACH_MODE_UNTIL/FOR_EACH_WIDER_MODE
> > etc. wouldn't work at all if GET_MODE_WIDER_MODE (BFmode) == SFmode
> > && GET_MODE_WIDER_MODE (HFmode) == SFmode.
>
> Yes, it seems they need to change now that their assumptions have been
> violated.  I suppose FOR_EACH_MODE_IN_CLASS would need to change to not use
> get_wider, and users of FOR_EACH_MODE/FOR_EACH_MODE_UNTIL need to decide
> whether they want an iteration that uses get_wider (likely with a new name)
> or not.

Here is a patch which does that.

Though I admit I didn't go carefully through all 24 GET_MODE_WIDER_MODE
uses, 54 FOR_EACH_MODE_IN_CLASS uses, 3 FOR_EACH_MODE uses, 24
FOR_EACH_MODE_FROM, 6 FOR_EACH_MODE_UNTIL and 15 FOR_EACH_WIDER_MODE uses.
It is more important to go through the GET_MODE_WIDER_MODE and
FOR_EACH_WIDER_MODE uses because the patch changes behavior for those,
the rest keep their previous meaning and so can be changed incrementally
if the other meaning is desirable to them (I've of course changed the 3
spots I had to change in the previous BFmode patch and whatever triggered
during the bootstraps).

2022-10-13  Jakub Jelinek  <jakub@redhat.com>

	* genmodes.cc (emit_mode_wider): Emit previous content of
	mode_wider array into mode_next array and for mode_wider
	emit always VOIDmode for !CLASS_HAS_WIDER_MODES_P classes,
	otherwise skip through modes with the same precision.
	* machmode.h (mode_next): Declare.
	(GET_MODE_NEXT_MODE): New inline function.
	(mode_iterator::get_next, mode_iterator::get_known_next): New
	function templates.
	(FOR_EACH_MODE_IN_CLASS): Use get_next instead of get_wider.
	(FOR_EACH_MODE): Use get_known_next instead of get_known_wider.
	(FOR_EACH_MODE_FROM): Use get_next instead of get_wider.
	(FOR_EACH_WIDER_MODE_FROM): Define.
	(FOR_EACH_NEXT_MODE): Define.
	* expmed.cc (emit_store_flag_1): Use FOR_EACH_WIDER_MODE_FROM
	instead of FOR_EACH_MODE_FROM.
	* optabs.cc (prepare_cmp_insn): Likewise.  Remove redundant
	!CLASS_HAS_WIDER_MODES_P check.
	(prepare_float_lib_cmp): Use FOR_EACH_WIDER_MODE_FROM instead of
	FOR_EACH_MODE_FROM.
	* config/i386/i386-expand.cc (get_mode_wider_vector): Use
	GET_MODE_NEXT_MODE instead of GET_MODE_WIDER_MODE.