From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 86DA73858D37 for ; Mon, 22 May 2023 13:06:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 86DA73858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id B2D271FEB9; Mon, 22 May 2023 13:06:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1684760782; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BC40YH9k1BBax949XNYWf7B963bYg9XjFoDw9+CU2UM=; b=VfgJaHy/W237K8HSA/3Uu8Lze5J6KvuTER5Ul1xJoqWUn1Qo5sl/1Yi21wkvyT0VNo2Tsz zQUuSxGHlC2C4QycoaiF3fU42evcJa2EEdTbXGctmojy/zv157gnIq9brWXNpgY73qDkmE 7PEBdsTRu8pdZycT6Gz6iBSqtai43Qw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1684760782; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BC40YH9k1BBax949XNYWf7B963bYg9XjFoDw9+CU2UM=; b=QCI4mQ0qrmLniHscaHRtFbiURICW7ZmaF3cylVvraOruooCDjRptD9jnrQAxADVWG6fukz oFEuM/4wnoZk70Dg== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id A04CD2C141; Mon, 22 May 2023 13:06:22 +0000 (UTC) Date: Mon, 22 May 2023 13:06:22 +0000 (UTC) From: Richard Biener To: "Andre Vieira (lists)" cc: Richard Sandiford , Richard Biener , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH 2/3] Refactor widen_plus as internal_fn In-Reply-To: Message-ID: References: <51ce8969-3130-452e-092e-f9d91eff2dad@arm.com> <4df83136-82f9-de0b-7e66-007b9047174d@arm.com> <37abc128-776c-0f04-f755-30e514909e15@arm.com> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 18 May 2023, Andre Vieira (lists) wrote: > How about this? > > Not sure about the DEF_INTERNAL documentation I rewrote in internal-fn.def, > was struggling to word these, so improvements welcome! The even/odd variant optabs are also commutative_optab_p, so is the vec_widen_sadd without hi/lo or even/odd. +/* { dg-options "-O3 -save-temps -fdump-tree-vect-all" } */ do you really want -all? I think you want -details + else if (widening_fn_p (ifn) + || narrowing_fn_p (ifn)) + { + tree lhs = gimple_get_lhs (stmt); + if (!lhs) + { + error ("vector IFN call with no lhs"); + debug_generic_stmt (fn); that's an error because ...? Maybe we want to verify this for all ECF_CONST|ECF_NOTHROW (or pure instead of const) internal function calls, but I wouldn't add any verification as part of this patch (not special to widening/narrowing fns either). if (gimple_call_internal_p (stmt)) - return 0; + { + internal_fn fn = gimple_call_internal_fn (stmt); + switch (fn) + { + case IFN_VEC_WIDEN_PLUS_HI: + case IFN_VEC_WIDEN_PLUS_LO: + case IFN_VEC_WIDEN_MINUS_HI: + case IFN_VEC_WIDEN_MINUS_LO: + return 1; this now looks incomplete. I think that we want instead to have a default: returning 1 and then special-cases we want to cost as zero. Not sure which - maybe blame tells why this was added? I think we can deal with this as followup (likewise the ranger additions). Otherwise looks good to me. Thanks, Richard. > gcc/ChangeLog: > > 2023-04-25 Andre Vieira > Joel Hutton > Tamar Christina > > * config/aarch64/aarch64-simd.md (vec_widen_addl_lo_): > Rename > this ... > (vec_widen_add_lo_): ... to this. > (vec_widen_addl_hi_): Rename this ... > (vec_widen_add_hi_): ... to this. > (vec_widen_subl_lo_): Rename this ... > (vec_widen_sub_lo_): ... to this. > (vec_widen_subl_hi_): Rename this ... > (vec_widen_sub_hi_): ...to this. > * doc/generic.texi: Document new IFN codes. > * internal-fn.cc (ifn_cmp): Function to compare ifn's for > sorting/searching. > (lookup_hilo_internal_fn): Add lookup function. > (commutative_binary_fn_p): Add widen_plus fn's. > (widening_fn_p): New function. > (narrowing_fn_p): New function. > (direct_internal_fn_optab): Change visibility. > * internal-fn.def (DEF_INTERNAL_WIDENING_OPTAB_FN): Macro to define an > internal_fn that expands into multiple internal_fns for widening. > (DEF_INTERNAL_NARROWING_OPTAB_FN): Likewise but for narrowing. > (IFN_VEC_WIDEN_PLUS, IFN_VEC_WIDEN_PLUS_HI, IFN_VEC_WIDEN_PLUS_LO, > IFN_VEC_WIDEN_PLUS_EVEN, IFN_VEC_WIDEN_PLUS_ODD, > IFN_VEC_WIDEN_MINUS, IFN_VEC_WIDEN_MINUS_HI, > IFN_VEC_WIDEN_MINUS_LO, > IFN_VEC_WIDEN_MINUS_ODD, IFN_VEC_WIDEN_MINUS_EVEN): Define widening > plus,minus functions. > * internal-fn.h (direct_internal_fn_optab): Declare new prototype. > (lookup_hilo_internal_fn): Likewise. > (widening_fn_p): Likewise. > (Narrowing_fn_p): Likewise. > * optabs.cc (commutative_optab_p): Add widening plus optabs. > * optabs.def (OPTAB_D): Define widen add, sub optabs. > * tree-cfg.cc (verify_gimple_call): Add checks for widening ifns. > * tree-inline.cc (estimate_num_insns): Return same > cost for widen add and sub IFNs as previous tree_codes. > * tree-vect-patterns.cc (vect_recog_widen_op_pattern): Support > patterns with a hi/lo or even/odd split. > (vect_recog_sad_pattern): Refactor to use new IFN codes. > (vect_recog_widen_plus_pattern): Likewise. > (vect_recog_widen_minus_pattern): Likewise. > (vect_recog_average_pattern): Likewise. > * tree-vect-stmts.cc (vectorizable_conversion): Add support for > _HILO IFNs. > (supportable_widening_operation): Likewise. > * tree.def (WIDEN_SUM_EXPR): Update example to use new IFNs. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/vect-widen-add.c: Test that new > IFN_VEC_WIDEN_PLUS is being used. > * gcc.target/aarch64/vect-widen-sub.c: Test that new > IFN_VEC_WIDEN_MINUS is being used. > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)