From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64426 invoked by alias); 18 Sep 2018 02:49:12 -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 64383 invoked by uid 89); 18 Sep 2018 02:49:08 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=regsh, newlib, rare, UD:regs.h X-HELO: arjuna.pair.com Received: from arjuna.pair.com (HELO arjuna.pair.com) (209.68.5.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Sep 2018 02:49:05 +0000 Received: by arjuna.pair.com (Postfix, from userid 3006) id BB5558A56E; Mon, 17 Sep 2018 22:48:52 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by arjuna.pair.com (Postfix) with ESMTP id BADFC8A43F for ; Mon, 17 Sep 2018 22:48:52 -0400 (EDT) Date: Tue, 18 Sep 2018 03:28:00 -0000 From: Hans-Peter Nilsson To: gcc-patches@gcc.gnu.org Subject: Committed, MMIX: don't expand __builtin_ffs to ffs Message-ID: User-Agent: Alpine 2.20.16 (BSF 172 2016-09-29) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00951.txt.bz2 I tried to update newlib (from an old copy from couple of years ago), but got curious regressions localized to tests related to ffs handling: --- regress.prev 2018-09-13 18:49:04.130070398 +0200 +++ regress 2018-09-13 22:59:25.551637529 +0200 @@ -0,0 +1,5 @@ +gcc.sum gcc.c-torture/execute/builtin-bitops-1.c +gcc.sum gcc.c-torture/execute/ffs-1.c +gcc.sum gcc.c-torture/execute/ffs-2.c +gcc.sum gcc.c-torture/execute/pr61725.c +gcc.sum gcc.dg/pr85376.c The added comment above mmix_init_libfuncs says the most, but see also the special case in optab-libfuncs.c:init_optabs. There's no __ffssi2 in libgcc for "64-bit targets" (__ffsSI2 becomes __ffsdi2). This may not be worthwile of more elegant generic handling, but to do this properly, the middle-end would have to open-code promotion of arguments and call __ffsdi2 for __builtin_ffs instead of the cop-out of forwarding to ffs, which is bound to break similarly for other targets. I guess no other target has seen this only because LP64 targets with neither ffs nor leading nor trailing zeros instructions are rare. I intend to back-port this to the gcc-8-branch too, given the usual freedom for non-primary-or-secondary targets (not strictly a regression). The mentioned regressions were fixed with this; committed. gcc: Handle a library implementation of ffs calling __builtin_ffs. * config/mmix/mmix.c (TARGET_INIT_LIBFUNCS): Override with... (mmix_init_libfuncs): New function: make __builtin_ffs expand to __ffsdi2. Index: gcc/config/mmix/mmix.c =================================================================== --- gcc/config/mmix/mmix.c (revision 264350) +++ gcc/config/mmix/mmix.c (working copy) @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. #include "memmodel.h" #include "tm_p.h" #include "insn-config.h" +#include "optabs.h" #include "regs.h" #include "emit-rtl.h" #include "recog.h" @@ -140,6 +141,7 @@ static void mmix_setup_incoming_varargs (cumulative_args_t, machine_mode, tree, int *, int); static void mmix_file_start (void); static void mmix_file_end (void); +static void mmix_init_libfuncs (void); static bool mmix_rtx_costs (rtx, machine_mode, int, int, int *, bool); static int mmix_register_move_cost (machine_mode, reg_class_t, reg_class_t); @@ -221,6 +223,9 @@ static HOST_WIDE_INT mmix_starting_frame #undef TARGET_ASM_OUTPUT_SOURCE_FILENAME #define TARGET_ASM_OUTPUT_SOURCE_FILENAME mmix_asm_output_source_filename +#undef TARGET_INIT_LIBFUNCS +#define TARGET_INIT_LIBFUNCS mmix_init_libfuncs + #undef TARGET_CONDITIONAL_REGISTER_USAGE #define TARGET_CONDITIONAL_REGISTER_USAGE mmix_conditional_register_usage @@ -1308,6 +1313,20 @@ mmix_asm_output_source_filename (FILE *s fprintf (stream, "\n"); } +/* Unfortunately, by default __builtin_ffs is expanded to ffs for + targets where INT_TYPE_SIZE < BITS_PER_WORD. That together with + newlib since 2017-07-04 implementing ffs as __builtin_ffs leads to + (newlib) ffs recursively calling itself. But, because of argument + promotion, and with ffs we're counting from the least bit, the + libgcc equivalent for ffsl works equally well for int arguments, so + just use that. */ + +static void +mmix_init_libfuncs (void) +{ + set_optab_libfunc (ffs_optab, SImode, "__ffsdi2"); +} + /* OUTPUT_QUOTED_STRING. */ void brgds, H-P