From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 25F483833005 for ; Wed, 18 Nov 2020 21:45:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 25F483833005 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-63-t-RJ3absNfK_0gYOJptYZw-1; Wed, 18 Nov 2020 16:45:48 -0500 X-MC-Unique: t-RJ3absNfK_0gYOJptYZw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 50DBC1868400; Wed, 18 Nov 2020 21:45:46 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-176.phx2.redhat.com [10.3.112.176]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6DE9F5D6A1; Wed, 18 Nov 2020 21:45:44 +0000 (UTC) Subject: Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193] To: Richard Biener Cc: gcc-patches@gcc.gnu.org, segher@kernel.crashing.org, joseph@codesourcery.com, jakub@redhat.com, hp@bitrange.com, will_schmidt@vnet.ibm.com References: <20201103231150.zlqccshb3qw63bdv@work-tp> <20201104151049.psotxu7xarcxmv5g@work-tp> <1c09d2bc-da08-523a-709d-ec11140af3f0@redhat.com> From: Jeff Law Message-ID: <3cda9726-6bdf-05bf-4168-42d3746a1665@redhat.com> Date: Wed, 18 Nov 2020 14:45:44 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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 21:45:51 -0000 On 11/18/20 12:31 AM, Richard Biener wrote: > 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). Yes.  But I think we end up doing that mostly for expanders that return the object where the value was stored in some reasonably convenient location (either as a return value or in an ops array).  I don't think that's the case here.  Jeff