From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 48AA3385800A for ; Wed, 18 Nov 2020 07:31:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 48AA3385800A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 565D8ABF4; Wed, 18 Nov 2020 07:31:29 +0000 (UTC) Date: Wed, 18 Nov 2020 08:31:28 +0100 (CET) From: Richard Biener To: Jeff Law cc: gcc-patches@gcc.gnu.org, segher@kernel.crashing.org, joseph@codesourcery.com, jakub@redhat.com, hp@bitrange.com, will_schmidt@vnet.ibm.com Subject: Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193] In-Reply-To: <1c09d2bc-da08-523a-709d-ec11140af3f0@redhat.com> Message-ID: References: <20201103231150.zlqccshb3qw63bdv@work-tp> <20201104151049.psotxu7xarcxmv5g@work-tp> <1c09d2bc-da08-523a-709d-ec11140af3f0@redhat.com> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Nov 2020 07:31:31 -0000 On Tue, 17 Nov 2020, Jeff Law wrote: > > > On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote: > > On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote: > >>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the > >>> + result and setting it in TARGET. Otherwise return NULL_RTX on failure. */ > >>> +static rtx > >>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode) > >>> +{ > >>> + if (!validate_arglist (exp, VOID_TYPE)) > >>> + return NULL_RTX; > >>> + > >>> + insn_code icode = direct_optab_handler (fegetround_optab, SImode); > >>> + if (icode == CODE_FOR_nothing) > >>> + return NULL_RTX; > >>> + > >>> + if (target == 0 > >>> + || GET_MODE (target) != target_mode > >>> + || !(*insn_data[icode].operand[0].predicate) (target, target_mode)) > >>> + target = gen_reg_rtx (target_mode); > >>> + > >>> + rtx pat = GEN_FCN (icode) (target); > >>> + if (!pat) > >>> + return NULL_RTX; > >>> + emit_insn (pat); > >> I think you need to verify whether the expansion ended up in 'target' > >> and otherwise emit a move since usually 'target' is just a hint. > > I thought the "if (target == 0 ..." took care of that. The expands do > > emit a move, if that helps. > It looks like if we have a passed in target and it either has the wrong > mode or it does not match the predicate, then we generaet a new target > and use that instead.? I don't see where we'd copy from that new target > to the original desired target.? For some expanders the caller would > handle that, but I don't see how that's possible for this one without > the caller digging into the generated RTL to determine that > expand_builtin_fegetround put the result somewhere other than TARGET and > thus a copy is needed. > > That may be what Richi is worried about. I know we've added missing if (!rtx_equal_p (target, ops[0].value)) emit_move_insn (target, ops[0].value); to several expanders (using expand_insn rather than manual GEN_FCN (icode) calls). Richard.