From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27036 invoked by alias); 28 Apr 2014 09:46:10 -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 27026 invoked by uid 89); 28 Apr 2014 09:46:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f181.google.com Received: from mail-vc0-f181.google.com (HELO mail-vc0-f181.google.com) (209.85.220.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 28 Apr 2014 09:46:08 +0000 Received: by mail-vc0-f181.google.com with SMTP id hy4so1798180vcb.26 for ; Mon, 28 Apr 2014 02:46:06 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.220.81.194 with SMTP id y2mr719924vck.29.1398678366179; Mon, 28 Apr 2014 02:46:06 -0700 (PDT) Received: by 10.52.78.194 with HTTP; Mon, 28 Apr 2014 02:46:06 -0700 (PDT) In-Reply-To: <535B9132.40703@linaro.org> References: <535B9132.40703@linaro.org> Date: Mon, 28 Apr 2014 10:03:00 -0000 Message-ID: Subject: Re: [RFC][AARCH64] TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook From: Marcus Shawcroft To: Kugan Cc: "gcc-patches@gcc.gnu.org" , Marcus Shawcroft , Richard Earnshaw Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg01828.txt.bz2 Hi Kugan, Thanks for this, couple of comments inline: On 26 April 2014 11:57, Kugan wrote: > gcc/ > +2014-04-27 Kugan Vivekanandarajah > + > + * config/aarch64/aarch64.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New > + define. > + * config/aarch64/aarch64-builtins.c (arm_builtins) : Add aarch64_builtins ? > + AARCH64_BUILTIN_LDFPSCR and AARCH64_BUILTIN_STFPSCR. AArch32 has the traditional combined FPSCR, but AArch64 splits this register into FPSR and FPCR therefore I think AARCH64_BUILTIN_GET_FPCR and AARCH64_BUILTIN_SET_FPCR are more appropriate names. Likewise subsequent references to FPSCR in this patch should change to FPCR. > + (aarch64_init_builtins) : Initialize builtins > + __builtins_aarch64_stfpscr and __builtins_aarch64_ldfpscr. > + (aarch64_expand_builtin) : Expand builtins __builtins_aarch64_stfpscr > + and __builtins_aarch64_ldfpscr. > + (aarch64_atomic_assign_expand_fenv): New function. > + * config/aarch64/aarch64.md (stfpscr): New pattern. > + (ldfpscr) : Likewise. > + (unspecv): Add UNSPECV_LDFPSCR and UNSPECV_STFPSCR. > + + aarch64_builtin_decls[AARCH64_BUILTIN_LDFPSCR] + = add_builtin_function ("__builtin_aarch64_ldfscr", ftype_ldfpscr, I'd prefer __builtin_aarch64_get_fpcr and __builtin_aarch64_set_fpcr. We should document them in doc/extend.texi + const unsigned HOST_WIDE_INT FE_ALL_EXCEPT = (FE_INVALID | FE_DIVBYZERO + | FE_OVERFLOW | FE_UNDERFLOW + | FE_INEXACT); Indentation is funny here.. + /* Genareate the equivalence of : Spelling. + tree fenv_var = create_tmp_var (unsigned_type_node, NULL); + tree ldfpscr = aarch64_builtin_decls[AARCH64_BUILTIN_LDFPSCR]; + tree stfpscr = aarch64_builtin_decls[AARCH64_BUILTIN_STFPSCR]; Move the declarations to the top of the function please. +void aarch64_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update); + Drop the argument names and relocate to aarch64-protos.h please. + UNSPECV_LDFPSCR ; load floating point status and control register. It isn't a status register, how about: UNSPECV_GET_FPCR ; Represent fetch of FPCR content. Cheers /Marcus