From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100417 invoked by alias); 23 Aug 2019 13:47:33 -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 100409 invoked by uid 89); 23 Aug 2019 13:47:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-9.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Aug 2019 13:47:31 +0000 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 8651828; Fri, 23 Aug 2019 06:47:29 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.99.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E66923F718; Fri, 23 Aug 2019 06:47:28 -0700 (PDT) From: Richard Sandiford To: Ilya Leoshkevich Mail-Followup-To: Ilya Leoshkevich ,gcc-patches@gcc.gnu.org, segher@kernel.crashing.org, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org, segher@kernel.crashing.org Subject: Re: [PATCH v2 3/9] Introduce can_vector_compare_p function References: <20190822134551.18924-1-iii@linux.ibm.com> <20190822134551.18924-4-iii@linux.ibm.com> <20ABE51C-BEA0-4A58-9394-7948722C69B6@linux.ibm.com> Date: Fri, 23 Aug 2019 14:23:00 -0000 In-Reply-To: <20ABE51C-BEA0-4A58-9394-7948722C69B6@linux.ibm.com> (Ilya Leoshkevich's message of "Fri, 23 Aug 2019 13:26:57 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg01652.txt.bz2 Ilya Leoshkevich writes: >> Am 23.08.2019 um 12:43 schrieb Richard Sandiford : >> >> Ilya Leoshkevich writes: >>> @@ -3819,6 +3820,82 @@ can_compare_p (enum rtx_code code, machine_mode mode, >>> return 0; >>> } >>> >>> +/* can_vector_compare_p presents fake rtx binary operations to the the back-end >>> + in order to determine its capabilities. In order to avoid creating fake >>> + operations on each call, values from previous calls are cached in a global >>> + cached_binops hash_table. It contains rtxes, which can be looked up using >>> + binop_keys. */ >>> + >>> +struct binop_key { >>> + enum rtx_code code; /* Operation code. */ >>> + machine_mode value_mode; /* Result mode. */ >>> + machine_mode cmp_op_mode; /* Operand mode. */ >>> +}; >>> + >>> +struct binop_hasher : pointer_hash_mark, ggc_cache_remove { >>> + typedef rtx value_type; >>> + typedef binop_key compare_type; >>> + >>> + static hashval_t >>> + hash (enum rtx_code code, machine_mode value_mode, machine_mode cmp_op_mode) >>> + { >>> + inchash::hash hstate (0); >>> + hstate.add_int (code); >>> + hstate.add_int (value_mode); >>> + hstate.add_int (cmp_op_mode); >>> + return hstate.end (); >>> + } >>> + >>> + static hashval_t >>> + hash (const rtx &ref) >>> + { >>> + return hash (GET_CODE (ref), GET_MODE (ref), GET_MODE (XEXP (ref, 0))); >>> + } >>> + >>> + static bool >>> + equal (const rtx &ref1, const binop_key &ref2) >>> + { >>> + return (GET_CODE (ref1) == ref2.code) >>> + && (GET_MODE (ref1) == ref2.value_mode) >>> + && (GET_MODE (XEXP (ref1, 0)) == ref2.cmp_op_mode); >>> + } >>> +}; >>> + >>> +static GTY ((cache)) hash_table *cached_binops; >>> + >>> +static rtx >>> +get_cached_binop (enum rtx_code code, machine_mode value_mode, >>> + machine_mode cmp_op_mode) >>> +{ >>> + if (!cached_binops) >>> + cached_binops = hash_table::create_ggc (1024); >>> + binop_key key = { code, value_mode, cmp_op_mode }; >>> + hashval_t hash = binop_hasher::hash (code, value_mode, cmp_op_mode); >>> + rtx *slot = cached_binops->find_slot_with_hash (key, hash, INSERT); >>> + if (!*slot) >>> + *slot = gen_rtx_fmt_ee (code, value_mode, gen_reg_rtx (cmp_op_mode), >>> + gen_reg_rtx (cmp_op_mode)); >>> + return *slot; >>> +} >> >> Sorry, I didn't mean anything this complicated. I just meant that >> we should have a single cached rtx that we can change via PUT_CODE and >> PUT_MODE_RAW for each new query, rather than allocating a new rtx each >> time. >> >> Something like: >> >> static GTY ((cache)) rtx cached_binop; >> >> rtx >> get_cached_binop (machine_mode mode, rtx_code code, machine_mode op_mode) >> { >> if (cached_binop) >> { >> PUT_CODE (cached_binop, code); >> PUT_MODE_RAW (cached_binop, mode); >> PUT_MODE_RAW (XEXP (cached_binop, 0), op_mode); >> PUT_MODE_RAW (XEXP (cached_binop, 1), op_mode); >> } >> else >> { >> rtx reg1 = gen_raw_REG (op_mode, LAST_VIRTUAL_REGISTER + 1); >> rtx reg2 = gen_raw_REG (op_mode, LAST_VIRTUAL_REGISTER + 2); >> cached_binop = gen_rtx_fmt_ee (code, mode, reg1, reg2); >> } >> return cached_binop; >> } >> >> Thanks, >> Richard > > Oh, I must have completely missed the point: the cache is only for > storage, and stored values themselves don't really matter. > > To make rtx usable with GTY ((cache)) I had to do: > > --- a/gcc/rtl.h > +++ b/gcc/rtl.h > @@ -4427,4 +4427,9 @@ extern void gt_ggc_mx (rtx &); > extern void gt_pch_nx (rtx &); > extern void gt_pch_nx (rtx &, gt_pointer_operator, void *); > > +inline void > +gt_cleare_cache (rtx) > +{ > +} > + > #endif /* ! GCC_RTL_H */ > > Does that look ok? Ah, turns out I was thinking of "deletable" rather than "cache", sorry. > Another think that might turn out being important: in your first > suggestion you use > > if (insn_operand_predicate_fn pred = insn_data[icode].operand[3].predicate) > { > machine_mode cmp_mode = insn_data[icode].operand[3].mode; > > instead of simply insn_operand_matches - is there any difference? I guess it was premature optimisation: if the .md file doesn't specify a predicate, we don't even need to create the rtx. But since most targets probably do specify a predicate, using insn_matches is fine too. Thanks, Richard