From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101941 invoked by alias); 11 Nov 2015 10:18:09 -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 101931 invoked by uid 89); 11 Nov 2015 10:18:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f178.google.com Received: from mail-yk0-f178.google.com (HELO mail-yk0-f178.google.com) (209.85.160.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 11 Nov 2015 10:18:07 +0000 Received: by ykdr82 with SMTP id r82so41091914ykd.3 for ; Wed, 11 Nov 2015 02:18:05 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.129.13.215 with SMTP id 206mr9224266ywn.280.1447237085484; Wed, 11 Nov 2015 02:18:05 -0800 (PST) Received: by 10.37.93.11 with HTTP; Wed, 11 Nov 2015 02:18:05 -0800 (PST) In-Reply-To: <877flpmtsv.fsf@e105548-lin.cambridge.arm.com> References: <878u6aos0i.fsf@e105548-lin.cambridge.arm.com> <877flpmtsv.fsf@e105548-lin.cambridge.arm.com> Date: Wed, 11 Nov 2015 10:18:00 -0000 Message-ID: Subject: Re: Short-cut generation of simple built-in functions From: Richard Biener To: Richard Biener , GCC Patches , richard.sandiford@arm.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg01326.txt.bz2 On Tue, Nov 10, 2015 at 10:24 PM, Richard Sandiford wrote: > Richard Biener writes: >> On Sat, Nov 7, 2015 at 2:31 PM, Richard Sandiford >> wrote: >>> This patch short-circuits the builtins.c expansion code for a particular >>> gimple call if: >>> >>> - the function has an associated internal function >>> - the target implements that internal function >>> - the call has no side effects >>> >>> This allows a later patch to remove the builtins.c code, once calls with >>> side effects have been handled. >>> >>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi. >>> OK to install? >>> >>> Thanks, >>> Richard >>> >>> >>> gcc/ >>> * builtins.h (called_as_built_in): Declare. >>> * builtins.c (called_as_built_in): Make external. >>> * internal-fn.h (expand_internal_call): Define a variant that >>> specifies the internal function explicitly. >>> * internal-fn.c (expand_load_lanes_optab_fn) >>> (expand_store_lanes_optab_fn, expand_ANNOTATE, expand_GOMP_SIMD_LANE) >>> (expand_GOMP_SIMD_VF, expand_GOMP_SIMD_LAST_LANE) >>> (expand_GOMP_SIMD_ORDERED_START, expand_GOMP_SIMD_ORDERED_END) >>> (expand_UBSAN_NULL, expand_UBSAN_BOUNDS, expand_UBSAN_VPTR) >>> (expand_UBSAN_OBJECT_SIZE, expand_ASAN_CHECK, expand_TSAN_FUNC_EXIT) >>> (expand_UBSAN_CHECK_ADD, expand_UBSAN_CHECK_SUB) >>> (expand_UBSAN_CHECK_MUL, expand_ADD_OVERFLOW, expand_SUB_OVERFLOW) >>> (expand_MUL_OVERFLOW, expand_LOOP_VECTORIZED) >>> (expand_mask_load_optab_fn, expand_mask_store_optab_fn) >>> (expand_ABNORMAL_DISPATCHER, expand_BUILTIN_EXPECT, expand_VA_ARG) >>> (expand_UNIQUE, expand_GOACC_DIM_SIZE, expand_GOACC_DIM_POS) >>> (expand_GOACC_LOOP, expand_GOACC_REDUCTION, expand_direct_optab_fn) >>> (expand_unary_optab_fn, expand_binary_optab_fn): Add an internal_fn >>> argument. >>> (internal_fn_expanders): Update prototype. >>> (expand_internal_call): Define a variant that specifies the >>> internal function explicitly. Use it to implement the previous >>> interface. >>> * cfgexpand.c (expand_call_stmt): Try to expand calls to built-in >>> functions as calls to internal functions. >>> >>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>> index f65011e..bbcc7dc3 100644 >>> --- a/gcc/builtins.c >>> +++ b/gcc/builtins.c >>> @@ -222,7 +222,7 @@ is_builtin_fn (tree decl) >>> of the optimization level. This means whenever a function is invoked with >>> its "internal" name, which normally contains the prefix "__builtin". */ >>> >>> -static bool >>> +bool >>> called_as_built_in (tree node) >>> { >>> /* Note that we must use DECL_NAME, not DECL_ASSEMBLER_NAME_SET_P since >>> diff --git a/gcc/builtins.h b/gcc/builtins.h >>> index 917eb90..1d00068 100644 >>> --- a/gcc/builtins.h >>> +++ b/gcc/builtins.h >>> @@ -50,6 +50,7 @@ extern struct target_builtins *this_target_builtins; >>> extern bool force_folding_builtin_constant_p; >>> >>> extern bool is_builtin_fn (tree); >>> +extern bool called_as_built_in (tree); >>> extern bool get_object_alignment_1 (tree, unsigned int *, >>> unsigned HOST_WIDE_INT *); >>> extern unsigned int get_object_alignment (tree); >>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c >>> index bfbc958..dc7d4f5 100644 >>> --- a/gcc/cfgexpand.c >>> +++ b/gcc/cfgexpand.c >>> @@ -2551,10 +2551,25 @@ expand_call_stmt (gcall *stmt) >>> return; >>> } >>> >>> + /* If this is a call to a built-in function and it has no effect other >>> + than setting the lhs, try to implement it using an internal function >>> + instead. */ >>> + decl = gimple_call_fndecl (stmt); >>> + if (gimple_call_lhs (stmt) >>> + && !gimple_vdef (stmt) >> >> I think you want && ! gimple_has_side_effects (stmt) >> instead of checking !gimple_vdef (stmt). > > OK, I can do that, but what would the difference be in practice for > these types of call? I.e. are there cases for built-ins where: > > (A) gimple_vdef (stmt) && !gimple_side_effects (stmt) > > or: > > (B) !gimple_vdef (stmt) && gimple_side_effects (stmt) > > ? There was talk to make calls use volatile to prevent CSE and friends. Using gimple_has_side_effects is just the better check. > It just seems like this check should be the opposite of the one used > in the call-cdce patch (when deciding whether to optimise a call > with an lhs). In order to keep them in sync I'd need to use > gimple_side_effects rather than gimple_vdef there too, but is > (B) a possibility there? Not sure if the tests should be in-sync. I'm also not sure what you really want to check with >>> + /* If this is a call to a built-in function and it has no effect other >>> + than setting the lhs, try to implement it using an internal function >>> + instead. */ >>> + decl = gimple_call_fndecl (stmt); >>> + if (gimple_call_lhs (stmt) >>> + && !gimple_vdef (stmt) I know you want to catch errno setting here but shouldn't that use a different kind of check and only done for a specific subset of functions? For example do you want to replace sqrt() with an IFN in case it has a VUSE (it will have that with -frounding-math)? In this case you'll lose the implicit dependence on fesetround and friends. This implicit dependence is not checked by gimple_has_side_effects either btw. >>> + && (optimize || (decl && called_as_built_in (decl)))) >>> + { >>> + internal_fn ifn = replacement_internal_fn (stmt); >>> + if (ifn != IFN_LAST) >>> + { >>> + expand_internal_call (ifn, stmt); >>> + return; >>> + } >>> + } >>> + >>> exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3); >>> >>> CALL_EXPR_FN (exp) = gimple_call_fn (stmt); >>> - decl = gimple_call_fndecl (stmt); >>> builtin_p = decl && DECL_BUILT_IN (decl); >>> >>> /* If this is not a builtin function, the function type through which the >>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c >>> index 9f9f9cf..c03c0fc 100644 >>> --- a/gcc/internal-fn.c >>> +++ b/gcc/internal-fn.c >>> @@ -103,7 +103,7 @@ get_multi_vector_move (tree array_type, >> convert_optab optab) >>> /* Expand LOAD_LANES call STMT using optab OPTAB. */ >>> >>> static void >>> -expand_load_lanes_optab_fn (gcall *stmt, convert_optab optab) >>> +expand_load_lanes_optab_fn (internal_fn, gcall *stmt, convert_optab optab) >> >> I'm somewhat confused by all these unused internal_fn args. I'm sure >> we can avoid them? > > They're needed for: > >>> -/* Expand call STMT using OPTAB, which has a single output operand and >>> - NARGS input operands. */ >>> +/* Expand a call to FN using the operands in STMT. FN has a single >>> + output operand and NARGS input operands. */ >>> >>> static void >>> -expand_direct_optab_fn (gcall *stmt, direct_optab optab, unsigned int nargs) >>> +expand_direct_optab_fn (internal_fn fn, gcall *stmt, direct_optab optab, >>> + unsigned int nargs) >>> { >>> expand_operand *ops = XALLOCAVEC (expand_operand, nargs + 1); >>> >>> - internal_fn fn = gimple_call_internal_fn (stmt); >>> tree_pair types = direct_internal_fn_types (fn, stmt); >>> insn_code icode = direct_optab_handler (optab, TYPE_MODE (types.first)); > > where we previously assumed that the function was the same as the call. Ah, ok. Thanks, Richard. > Thanks, > Richard >