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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 6CE993858006 for ; Tue, 17 Nov 2020 22:23:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6CE993858006 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-234-cL5nHhRBOG6koAqsQi6oOA-1; Tue, 17 Nov 2020 17:23:04 -0500 X-MC-Unique: cL5nHhRBOG6koAqsQi6oOA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8751610074B1; Tue, 17 Nov 2020 22:23:03 +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 8814119C78; Tue, 17 Nov 2020 22:23:02 +0000 (UTC) Subject: Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193] To: Richard Biener , 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> From: Jeff Law Message-ID: <0341a102-6c1d-ceb1-ff4a-c9857ad2ec0f@redhat.com> Date: Tue, 17 Nov 2020 15:23:02 -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: <20201104151049.psotxu7xarcxmv5g@work-tp> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 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_H4, 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: Tue, 17 Nov 2020 22:23:08 -0000 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. > > For feclearexcept and feraiseexcept I included tests to variable > 'target', including none, but now I see that I did not do the same for > fegetround, I can add the same if it is necessary, but the test do check > if the return is correct, so I don't know. > > >>> +@cindex @code{fegetround@var{m}} instruction pattern >>> +@item @samp{fegetround@var{m}} >>> +Store the current machine floating-point rounding mode into operand 0. >>> +Operand 0 has mode @var{m}, which is scalar. This pattern is used to >>> +implement the @code{fegetround} function from the ISO C99 standard. >> I think this needs to elaborate on the format of the "rounding mode". >> >> AFAICS you do nothing to marshall with the actually used libc >> implementation which AFAIU can choose arbitrary values for >> the FE_* macros. I'm not sure we require the compiler to be >> configured for one specific C library and for example require >> matching FE_* macro definitions for all uses of the built >> compiler. >> >> For the patch at hand you seem to assume the libc "format" >> matches the hardware one (which would of course be reasonable). >> >> Does that actually hold up when looking at libcs other than >> glibc supporting powerpc? > I checked in some other libc implementations that have POWER support and > all have the same value as glic for the four rounding modes and the five > exception flags from libc. The libcs implementations I checked are: > > - musl > - uclibc & uclibc-ng > - freebsd > > Is There any other I am missing? I think the concern here is that while the libcs we have visibility into have consistent values, I don't think that's necessarily guaranteed.   I'm not sure how to DTRT here.  We may not have the target headers if we're doing a cross compile, so a configure test may not do what we  need.  In fact, ISTM that there is no reliable configure or compile time check we can do since the constants are part of the runtime and can change independently of the compiler. Jeff