From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106527 invoked by alias); 11 Aug 2017 17:30:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 104196 invoked by uid 89); 11 Aug 2017 17:30:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Aug 2017 17:30:00 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A3BD02DA982; Fri, 11 Aug 2017 17:29:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A3BD02DA982 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=law@redhat.com Received: from localhost.localdomain (ovpn-116-95.phx2.redhat.com [10.3.116.95]) by smtp.corp.redhat.com (Postfix) with ESMTP id 574815D9C7; Fri, 11 Aug 2017 17:29:58 +0000 (UTC) Subject: Re: [04/77] Add FOR_EACH iterators for modes To: gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org References: <8760ewohsv.fsf@linaro.org> <87o9son32j.fsf@linaro.org> <87tw1eqb3d.fsf@linaro.org> From: Jeff Law Message-ID: <41f9f8bb-b748-71b4-de2a-da89f09a9419@redhat.com> Date: Fri, 11 Aug 2017 17:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <87tw1eqb3d.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00807.txt.bz2 On 08/11/2017 11:16 AM, Richard Sandiford wrote: > Hi Jeff, > > First, many thanks for the reviews! > > Jeff Law writes: >> On 07/13/2017 02:39 AM, Richard Sandiford wrote: >>> The new iterators are: >>> >>> - FOR_EACH_MODE_IN_CLASS: iterate over all the modes in a mode class. >>> >>> - FOR_EACH_MODE_FROM: iterate over all the modes in a class, >>> starting at a given mode. >>> >>> - FOR_EACH_WIDER_MODE: iterate over all the modes in a class, >>> starting at the next widest mode after a given mode. >>> >>> - FOR_EACH_2XWIDER_MODE: same, but considering only modes that >>> are two times wider than the previous mode. >>> >>> - FOR_EACH_MODE_UNTIL: iterate over all the modes in a class until >>> a given mode is reached. >>> >>> - FOR_EACH_MODE: iterate over all the modes in a class between >>> two given modes, inclusive of the first but not the second. >>> >>> These help with the stronger type checking added by later patches, >>> since every new mode will be in the same class as the previous one. >>> >>> 2017-07-13 Richard Sandiford >>> Alan Hayward >>> David Sherwood >>> >>> gcc/ >>> * machmode.h (mode_traits): New structure. >>> (get_narrowest_mode): New function. >>> (mode_iterator::start): Likewise. >>> (mode_iterator::iterate_p): Likewise. >>> (mode_iterator::get_wider): Likewise. >>> (mode_iterator::get_known_wider): Likewise. >>> (mode_iterator::get_2xwider): Likewise. >>> (FOR_EACH_MODE_IN_CLASS): New mode iterator. >>> (FOR_EACH_MODE): Likewise. >>> (FOR_EACH_MODE_FROM): Likewise. >>> (FOR_EACH_MODE_UNTIL): Likewise. >>> (FOR_EACH_WIDER_MODE): Likewise. >>> (FOR_EACH_2XWIDER_MODE): Likewise. >>> * builtins.c (expand_builtin_strlen): Use new mode iterators. >>> * combine.c (simplify_comparison): Likewise >>> * config/i386/i386.c (type_natural_mode): Likewise. >>> * cse.c (cse_insn): Likewise. >>> * dse.c (find_shift_sequence): Likewise. >>> * emit-rtl.c (init_derived_machine_modes): Likewise. >>> (init_emit_once): Likewise. >>> * explow.c (hard_function_value): Likewise. >>> * expmed.c (init_expmed_one_mode): Likewise. >>> (extract_fixed_bit_field_1): Likewise. >>> (extract_bit_field_1): Likewise. >>> (expand_divmod): Likewise. >>> (emit_store_flag_1): Likewise. >>> * expr.c (init_expr_target): Likewise. >>> (convert_move): Likewise. >>> (alignment_for_piecewise_move): Likewise. >>> (widest_int_mode_for_size): Likewise. >>> (emit_block_move_via_movmem): Likewise. >>> (copy_blkmode_to_reg): Likewise. >>> (set_storage_via_setmem): Likewise. >>> (compress_float_constant): Likewise. >>> * omp-low.c (omp_clause_aligned_alignment): Likewise. >>> * optabs-query.c (get_best_extraction_insn): Likewise. >>> * optabs.c (expand_binop): Likewise. >>> (expand_twoval_unop): Likewise. >>> (expand_twoval_binop): Likewise. >>> (widen_leading): Likewise. >>> (widen_bswap): Likewise. >>> (expand_parity): Likewise. >>> (expand_unop): Likewise. >>> (prepare_cmp_insn): Likewise. >>> (prepare_float_lib_cmp): Likewise. >>> (expand_float): Likewise. >>> (expand_fix): Likewise. >>> (expand_sfix_optab): Likewise. >>> * postreload.c (move2add_use_add2_insn): Likewise. >>> * reg-stack.c (reg_to_stack): Likewise. >>> * reginfo.c (choose_hard_reg_mode): Likewise. >>> * rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise. >>> * stmt.c (emit_case_decision_tree): Likewise. >>> * stor-layout.c (mode_for_size): Likewise. >>> (smallest_mode_for_size): Likewise. >>> (mode_for_vector): Likewise. >>> (finish_bitfield_representative): Likewise. >>> * tree-ssa-math-opts.c (target_supports_divmod_p): Likewise. >>> * tree-vect-generic.c (type_for_widest_vector_mode): Likewise. >>> * tree-vect-stmts.c (vectorizable_conversion): Likewise. >>> * var-tracking.c (prepare_call_arguments): Likewise. >>> >>> gcc/ada/ >>> * gcc-interface/misc.c (fp_prec_to_size): Use new mode iterators. >>> (fp_size_to_prec): Likewise. >>> >>> gcc/c-family/ >>> * c-common.c (c_common_fixed_point_type_for_size): Use new mode >>> iterators. >>> * c-cppbuiltin.c (c_cpp_builtins): Likewise. >> So in some cases I see that we have loops like this: >> >> for (; mode != VOIDmode; mode = GET_WIDER_MODE (mode)) >> >> Which iterates from the current mode through all the remaining modes, >> with each iteration using a wider mode than the previous iteration. >> GET_WIDER_MODE is always going to give us something in the same class or >> VOIDmode. >> >> This is getting translated into: >> >> FOR_EACH_MODE_FROM (mode, mode) >> >> The two loops have different steps. The original steps to a wider mode >> each iteration. The new one may step to a different mode of the same >> width IIUC. >> >> Am I missing something here? > > The new iterators use GET_MODE_WIDER or GET_MODE_2XWIDER to step, > and the patch is only supposed to convert loops that have matching steps. > It wasn't supposed to convert loops that step through modes as numerical > values, but I see now that I fluffed it with: Ah, I was just doing a high level looksie at the conversions first using the descriptions you gave when I saw what appeared to be the inconsistency. Let me repeat that knowing that we're stepping with GET_MODE_WIDER and friends. > > Index: gcc/expmed.c > =================================================================== > --- gcc/expmed.c 2017-07-13 09:17:39.972572368 +0100 > +++ gcc/expmed.c 2017-07-13 09:18:21.531429258 +0100 > @@ -204,8 +204,7 @@ init_expmed_one_mode (struct init_expmed > > if (SCALAR_INT_MODE_P (mode)) > { > - for (mode_from = MIN_MODE_INT; mode_from <= MAX_MODE_INT; > - mode_from = (machine_mode)(mode_from + 1)) > + FOR_EACH_MODE_IN_CLASS (mode_from, MODE_INT) > init_expmed_one_conv (all, mode, mode_from, speed); > } > if (GET_MODE_CLASS (mode) == MODE_INT) > > Sorry about that! Will fix. > > [Maybe in this case the change is safe, since I'm not sure we support > multiple full-integer modes with the same width. It's still too subtle > for a mechanical change like this though.] > > Is that patch OK with that hunk removed? Let me walk through a few uses again, then the implementation of the iterators. Odds are it's fine, but I should look at it deeper than I have so far. jeff