From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 9D2ED3858D1E for ; Tue, 25 Apr 2023 12:30:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9D2ED3858D1E 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-out1.suse.de (Postfix) with ESMTP id C032221985; Tue, 25 Apr 2023 12:30:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1682425801; 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=VrqKOFqy0t38fW5b1WPcztCbAokB10guwhiTddckLkI=; b=sY2LLl+tEg8arfo0oO5GZUPUMNIuMEuCO/nLjW/Cfq7cwXQPEC4vJcUfFm9j7Jh09/2I0S AGE94MXSo0j+wVsGpusVwBtJ6JdqjgtJlZZF2kml/stNhl6IBoim/HJirIjX55HtHPxp0T nMtrIY1nqia01zNcGIeuZFtpcY/Y6uk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1682425801; 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=VrqKOFqy0t38fW5b1WPcztCbAokB10guwhiTddckLkI=; b=zypl0R/q3uRX11WQ/UlObgPIgcCxFREl3k1AICIfNBSY3klv1hBSyMj/O4zv30tpKXP1Ox YP60Tc8RC2ndT3BA== 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 A088D2C141; Tue, 25 Apr 2023 12:30:01 +0000 (UTC) Date: Tue, 25 Apr 2023 12:30:01 +0000 (UTC) From: Richard Biener To: Richard Sandiford cc: Richard Biener , "Andre Vieira (lists)" , "gcc-patches@gcc.gnu.org" Subject: Re: [ping][vect-patterns] Refactor widen_plus/widen_minus as internal_fns In-Reply-To: Message-ID: References: <51ce8969-3130-452e-092e-f9d91eff2dad@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,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 Mon, 24 Apr 2023, Richard Sandiford wrote: > Richard Biener writes: > > On Thu, Apr 20, 2023 at 3:24?PM Andre Vieira (lists) via Gcc-patches > > wrote: > >> > >> Rebased all three patches and made some small changes to the second one: > >> - removed sub and abd optabs from commutative_optab_p, I suspect this > >> was a copy paste mistake, > >> - removed what I believe to be a superfluous switch case in vectorizable > >> conversion, the one that was here: > >> + if (code.is_fn_code ()) > >> + { > >> + internal_fn ifn = as_internal_fn (code.as_fn_code ()); > >> + int ecf_flags = internal_fn_flags (ifn); > >> + gcc_assert (ecf_flags & ECF_MULTI); > >> + > >> + switch (code.as_fn_code ()) > >> + { > >> + case CFN_VEC_WIDEN_PLUS: > >> + break; > >> + case CFN_VEC_WIDEN_MINUS: > >> + break; > >> + case CFN_LAST: > >> + default: > >> + return false; > >> + } > >> + > >> + internal_fn lo, hi; > >> + lookup_multi_internal_fn (ifn, &lo, &hi); > >> + *code1 = as_combined_fn (lo); > >> + *code2 = as_combined_fn (hi); > >> + optab1 = lookup_multi_ifn_optab (lo, !TYPE_UNSIGNED (vectype)); > >> + optab2 = lookup_multi_ifn_optab (hi, !TYPE_UNSIGNED (vectype)); > >> } > >> > >> I don't think we need to check they are a specfic fn code, as we look-up > >> optabs and if they succeed then surely we can vectorize? > >> > >> OK for trunk? > > > > In the first patch I see some uses of safe_as_tree_code like > > > > + if (ch.is_tree_code ()) > > + return op1 == NULL_TREE ? gimple_build_assign (lhs, > > ch.safe_as_tree_code (), > > + op0) : > > + gimple_build_assign (lhs, ch.safe_as_tree_code (), > > + op0, op1); > > + else > > + { > > + internal_fn fn = as_internal_fn (ch.safe_as_fn_code ()); > > + gimple* stmt; > > > > where the context actually requires a valid tree code. Please change those > > to force to tree code / ifn code. Just use explicit casts here and the other > > places that are similar. Before the as_internal_fn just put a > > gcc_assert (ch.is_internal_fn ()). > > Also, doesn't the above ?: simplify to the "else" arm? Null trailing > arguments would be ignored for unary operators. > > I wasn't sure what to make of the op0 handling: > > > +/* Build a GIMPLE_ASSIGN or GIMPLE_CALL with the tree_code, > > + or internal_fn contained in ch, respectively. */ > > +gimple * > > +vect_gimple_build (tree lhs, code_helper ch, tree op0, tree op1) > > +{ > > + if (op0 == NULL_TREE) > > + return NULL; > > Can that happen, and if so, does returning null make sense? > Maybe an assert would be safer. Yeah, I was hoping to have a look whether the new gimple_build overloads could be used to make this all better (but hoped we can finally get this series in in some way). Richard.