From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 30E7E38515E4 for ; Wed, 12 Oct 2022 11:49:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 30E7E38515E4 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0CA4B113E; Wed, 12 Oct 2022 04:49:16 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DA9253F792; Wed, 12 Oct 2022 04:49:08 -0700 (PDT) From: Richard Sandiford To: Jakub Jelinek Mail-Followup-To: Jakub Jelinek ,Jason Merrill , Richard Biener , Jeff Law , Eric Botcazou , gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: Jason Merrill , Richard Biener , Jeff Law , Eric Botcazou , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] machmode, v2: Introduce GET_MODE_NEXT_MODE with previous GET_MODE_WIDER_MODE meaning, add new GET_MODE_WIDER_MODE References: <37522634-319a-b471-aa35-87e711b0479e@redhat.com> <55062a15-79a1-f8cf-ed20-25ca8ff42abe@redhat.com> <95f2abba-afb4-bb73-a9f0-b1578b28713a@redhat.com> Date: Wed, 12 Oct 2022 12:49:07 +0100 In-Reply-To: (Jakub Jelinek's message of "Wed, 12 Oct 2022 13:07:34 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-38.4 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Jakub Jelinek writes: > On Wed, Oct 12, 2022 at 11:15:40AM +0100, Richard Sandiford wrote: >> Looks good to me, just some minor comments below. > > Here is an updated patch. > >> How robust is the mechanism that guarantees HF comes before BF, >> and so is the mode that appears in the (new) wider list? > > genmodes.cc seems to have cmp_modes which does a lot of different > comparisons to make sure it is a total order. > I think the BFmode vs. HFmode ordering is about the last case: > if (m->counter < n->counter) > return -1; > else > return 1; > there because everything else is equal and ->counter is about which > mode is declared first in *-modes.def. > And my code for the new mode_wider in genmodes.cc always uses VOIDmode > for !CLASS_HAS_WIDER_MODES_P classes and for CLASS_HAS_WIDER_MODES_P > classes provides a subset of the total ordering, in the already computed > ->wider chain it skips modes with the same size/precision. OK, I guess that's good enough. > >> > - /* Set mode iterator *ITER to the next widest mode in the same class. >> > + /* Set mode iterator *ITER to the next wider mode in the same class. >> > Such a mode is known to exist. */ >> >> I'll take your word for it that this is correct. ;-) I would say >> "next widest", but it's very likely that I'm wrong. > > I'm not a native english speaker, but to me next with superlative would be > if we have the widest mode, next widest would be the one whose only > wider mode is the widest mode. > > Everything else changed. > > 2022-10-12 Jakub Jelinek > > * 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. LGTM, but please give others 24 hours to object. Thanks, Richard > --- gcc/genmodes.cc.jj 2022-10-12 10:15:21.444381490 +0200 > +++ gcc/genmodes.cc 2022-10-12 12:28:02.414528652 +0200 > @@ -1527,7 +1527,7 @@ emit_mode_wider (void) > int c; > struct mode_data *m; > > - print_decl ("unsigned char", "mode_wider", "NUM_MACHINE_MODES"); > + print_decl ("unsigned char", "mode_next", "NUM_MACHINE_MODES"); > > for_all_modes (c, m) > tagged_printf ("E_%smode", > @@ -1535,6 +1535,37 @@ emit_mode_wider (void) > m->name); > > print_closer (); > + print_decl ("unsigned char", "mode_wider", "NUM_MACHINE_MODES"); > + > + for_all_modes (c, m) > + { > + struct mode_data *m2 = 0; > + > + if (m->cl == MODE_INT > + || m->cl == MODE_PARTIAL_INT > + || m->cl == MODE_FLOAT > + || m->cl == MODE_DECIMAL_FLOAT > + || m->cl == MODE_COMPLEX_FLOAT > + || m->cl == MODE_FRACT > + || m->cl == MODE_UFRACT > + || m->cl == MODE_ACCUM > + || m->cl == MODE_UACCUM) > + for (m2 = m->wider; m2 && m2 != void_mode; m2 = m2->wider) > + { > + if (m2->bytesize == m->bytesize > + && m2->precision == m->precision) > + continue; > + break; > + } > + > + if (m2 == void_mode) > + m2 = 0; > + tagged_printf ("E_%smode", > + m2 ? m2->name : void_mode->name, > + m->name); > + } > + > + print_closer (); > print_decl ("unsigned char", "mode_2xwider", "NUM_MACHINE_MODES"); > > for_all_modes (c, m) > --- gcc/machmode.h.jj 2022-10-12 10:15:21.491380846 +0200 > +++ gcc/machmode.h 2022-10-12 12:33:36.795975117 +0200 > @@ -28,6 +28,7 @@ extern const unsigned char mode_inner[NU > extern CONST_MODE_NUNITS poly_uint16_pod mode_nunits[NUM_MACHINE_MODES]; > extern CONST_MODE_UNIT_SIZE unsigned char mode_unit_size[NUM_MACHINE_MODES]; > extern const unsigned short mode_unit_precision[NUM_MACHINE_MODES]; > +extern const unsigned char mode_next[NUM_MACHINE_MODES]; > extern const unsigned char mode_wider[NUM_MACHINE_MODES]; > extern const unsigned char mode_2xwider[NUM_MACHINE_MODES]; > > @@ -760,7 +761,23 @@ GET_MODE_NUNITS (const T &mode) > } > #endif > > -/* Get the next wider natural mode (eg, QI -> HI -> SI -> DI -> TI). */ > +/* Get the next natural mode (not narrower, eg, QI -> HI -> SI -> DI -> TI > + or HF -> BF -> SF -> DF -> XF -> TF). */ > + > +template > +ALWAYS_INLINE opt_mode > +GET_MODE_NEXT_MODE (const T &m) > +{ > + return typename opt_mode::from_int (mode_next[m]); > +} > + > +/* Get the next wider mode (eg, QI -> HI -> SI -> DI -> TI > + or { HF, BF } -> SF -> DF -> XF -> TF). > + This is similar to GET_MODE_NEXT_MODE, but while GET_MODE_NEXT_MODE > + can include mode that have the same precision (e.g. > + GET_MODE_NEXT_MODE (HFmode) can be BFmode even when both have the same > + precision), this one will skip those. And always VOIDmode for > + modes whose class is !CLASS_HAS_WIDER_MODES_P. */ > > template > ALWAYS_INLINE opt_mode > @@ -1098,7 +1115,33 @@ namespace mode_iterator > return *iter != E_VOIDmode; > } > > - /* Set mode iterator *ITER to the next widest mode in the same class, > + /* Set mode iterator *ITER to the next mode in the same class, > + if any. */ > + > + template > + inline void > + get_next (opt_mode *iter) > + { > + *iter = GET_MODE_NEXT_MODE (iter->require ()); > + } > + > + inline void > + get_next (machine_mode *iter) > + { > + *iter = GET_MODE_NEXT_MODE (*iter).else_void (); > + } > + > + /* Set mode iterator *ITER to the next mode in the same class. > + Such a mode is known to exist. */ > + > + template > + inline void > + get_known_next (T *iter) > + { > + *iter = GET_MODE_NEXT_MODE (*iter).require (); > + } > + > + /* Set mode iterator *ITER to the next wider mode in the same class, > if any. */ > > template > @@ -1114,7 +1157,7 @@ namespace mode_iterator > *iter = GET_MODE_WIDER_MODE (*iter).else_void (); > } > > - /* Set mode iterator *ITER to the next widest mode in the same class. > + /* Set mode iterator *ITER to the next wider mode in the same class. > Such a mode is known to exist. */ > > template > @@ -1146,20 +1189,27 @@ namespace mode_iterator > #define FOR_EACH_MODE_IN_CLASS(ITERATOR, CLASS) \ > for (mode_iterator::start (&(ITERATOR), CLASS); \ > mode_iterator::iterate_p (&(ITERATOR)); \ > - mode_iterator::get_wider (&(ITERATOR))) > + mode_iterator::get_next (&(ITERATOR))) > > /* Make ITERATOR iterate over all the modes in the range [START, END), > in order of increasing width. */ > #define FOR_EACH_MODE(ITERATOR, START, END) \ > for ((ITERATOR) = (START); \ > (ITERATOR) != (END); \ > - mode_iterator::get_known_wider (&(ITERATOR))) > + mode_iterator::get_known_next (&(ITERATOR))) > > -/* Make ITERATOR iterate over START and all wider modes in the same > +/* Make ITERATOR iterate over START and all non-narrower modes in the same > class, in order of increasing width. */ > #define FOR_EACH_MODE_FROM(ITERATOR, START) \ > for ((ITERATOR) = (START); \ > mode_iterator::iterate_p (&(ITERATOR)); \ > + mode_iterator::get_next (&(ITERATOR))) > + > +/* Make ITERATOR iterate over START and all wider modes in the same > + class, in order of strictly increasing width. */ > +#define FOR_EACH_WIDER_MODE_FROM(ITERATOR, START) \ > + for ((ITERATOR) = (START); \ > + mode_iterator::iterate_p (&(ITERATOR)); \ > mode_iterator::get_wider (&(ITERATOR))) > > /* Make ITERATOR iterate over modes in the range [NARROWEST, END) > @@ -1169,6 +1219,14 @@ namespace mode_iterator > FOR_EACH_MODE (ITERATOR, get_narrowest_mode (END), END) > > /* Make ITERATOR iterate over modes in the same class as MODE, in order > + of non-decreasing width. Start at next such mode after START, > + or don't iterate at all if there is no such mode. */ > +#define FOR_EACH_NEXT_MODE(ITERATOR, START) \ > + for ((ITERATOR) = (START), mode_iterator::get_next (&(ITERATOR)); \ > + mode_iterator::iterate_p (&(ITERATOR)); \ > + mode_iterator::get_next (&(ITERATOR))) > + > +/* Make ITERATOR iterate over modes in the same class as MODE, in order > of increasing width. Start at the first mode wider than START, > or don't iterate at all if there is no wider mode. */ > #define FOR_EACH_WIDER_MODE(ITERATOR, START) \ > --- gcc/expmed.cc.jj 2022-10-12 10:15:21.335382983 +0200 > +++ gcc/expmed.cc 2022-10-12 12:28:02.417528612 +0200 > @@ -5712,7 +5712,7 @@ emit_store_flag_1 (rtx target, enum rtx_ > > /* Next try expanding this via the backend's cstore4. */ > mclass = GET_MODE_CLASS (mode); > - FOR_EACH_MODE_FROM (compare_mode, mode) > + FOR_EACH_WIDER_MODE_FROM (compare_mode, mode) > { > machine_mode optab_mode = mclass == MODE_CC ? CCmode : compare_mode; > icode = optab_handler (cstore_optab, optab_mode); > --- gcc/optabs.cc.jj 2022-10-12 10:15:21.542380147 +0200 > +++ gcc/optabs.cc 2022-10-12 12:28:02.418528598 +0200 > @@ -4384,7 +4384,6 @@ prepare_cmp_insn (rtx x, rtx y, enum rtx > machine_mode mode = *pmode; > rtx libfunc, test; > machine_mode cmp_mode; > - enum mode_class mclass; > > /* The other methods are not needed. */ > gcc_assert (methods == OPTAB_DIRECT || methods == OPTAB_WIDEN > @@ -4490,9 +4489,8 @@ prepare_cmp_insn (rtx x, rtx y, enum rtx > return; > } > > - mclass = GET_MODE_CLASS (mode); > test = gen_rtx_fmt_ee (comparison, VOIDmode, x, y); > - FOR_EACH_MODE_FROM (cmp_mode, mode) > + FOR_EACH_WIDER_MODE_FROM (cmp_mode, mode) > { > enum insn_code icode; > icode = optab_handler (cbranch_optab, cmp_mode); > @@ -4515,7 +4513,7 @@ prepare_cmp_insn (rtx x, rtx y, enum rtx > delete_insns_since (last); > } > > - if (methods == OPTAB_DIRECT || !CLASS_HAS_WIDER_MODES_P (mclass)) > + if (methods == OPTAB_DIRECT) > break; > } > > @@ -4711,7 +4709,7 @@ prepare_float_lib_cmp (rtx x, rtx y, enu > bool reversed_p = false; > scalar_int_mode cmp_mode = targetm.libgcc_cmp_return_mode (); > > - FOR_EACH_MODE_FROM (mode, orig_mode) > + FOR_EACH_WIDER_MODE_FROM (mode, orig_mode) > { > if (code_to_optab (comparison) > && (libfunc = optab_libfunc (code_to_optab (comparison), mode))) > --- gcc/config/i386/i386-expand.cc.jj 2022-09-26 18:47:26.892350579 +0200 > +++ gcc/config/i386/i386-expand.cc 2022-10-12 12:28:02.421528557 +0200 > @@ -14941,7 +14941,7 @@ static machine_mode > get_mode_wider_vector (machine_mode o) > { > /* ??? Rely on the ordering that genmodes.cc gives to vectors. */ > - machine_mode n = GET_MODE_WIDER_MODE (o).require (); > + machine_mode n = GET_MODE_NEXT_MODE (o).require (); > gcc_assert (GET_MODE_NUNITS (o) == GET_MODE_NUNITS (n) * 2); > gcc_assert (GET_MODE_SIZE (o) == GET_MODE_SIZE (n)); > return n; > > > Jakub