From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x231.google.com (mail-lj1-x231.google.com [IPv6:2a00:1450:4864:20::231]) by sourceware.org (Postfix) with ESMTPS id 1A7C93830B70 for ; Sat, 22 Jun 2024 13:19:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1A7C93830B70 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1A7C93830B70 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::231 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719062381; cv=none; b=cWYTnyvPhdh3QOPh63TIHKYUUp2rrMXmNanJlSF+z7g1/+wf6UxhTIIrQ43wQQDFqM0zafU+jgoxuDqBFb5PbYXMN74gM7LxT92w8EuWxUdDETWuiSoMLv/1wBPdkgotS1JAiqtQ6SJGhO++RTcvv0GwdUPicsDUcHviZXNMYrI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719062381; c=relaxed/simple; bh=XIc4BvKBg1RYmWpirNqyWfVkx74asV2t6gZnlGMT1vU=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=jeXqtQ2RXFThzEaQ9o/CtoLbYFXoaXuMW7J+D/qtmy4HHbz/S1IKFX685jd0/G43GehaJAUuf9ky7c3y2qOzyjCAbzQZly5beWTnKPTF6QAvuufG1VMZNsaEnkushP4XDvGUxcEOaSSrw8jmfzLRR3eCzr419RXPiJLxzfO2y28= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lj1-x231.google.com with SMTP id 38308e7fff4ca-2ec002caf3eso41135061fa.1 for ; Sat, 22 Jun 2024 06:19:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719062377; x=1719667177; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Fd7Jp7Uy8AGRavxsE5//8z65Uq5Zqf5sGphbsJCwY5s=; b=W5y6iEYCd5KZFNdAYk3NWqcWAyGZtF/+Fq28nviwkr21lST3DqYm1SPVArvlaDmz6C uXt2M8vbUQ8cbqi1rVUlMdwomr1FZcXxoselRFMXH+2EPcPDnwWAmQGKqoqLe9zwDaH5 UeuvNRZqqz2EP4Nq9dsvNzerzbJuDz4W3AW89W3f1E4xmGOGcTXpYCcPzoZ9RhdhY5ow hdaNOwewk2D7lUTZfyo5rlcLLIU1n28Je6fY9LWTTLJhQaeCp+MSYQe1P7XvQwOVSr78 4jeUw4ML50+0PapVWmPCXYFDe4lduYKK4bR5g++3yhleZeo7Zi24h7uE8EtiU0oFo2zR IIwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719062377; x=1719667177; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Fd7Jp7Uy8AGRavxsE5//8z65Uq5Zqf5sGphbsJCwY5s=; b=uL1I3ZmguMQFpjsZV2a/7LJVsRLMyRoZJ007+zo4e9M5cIDzZiEA28Z5Itd9TPcghF ywMlnlaDi+DdzuxRDdmV4tGaoU2k/C8RieSgHOyegR7ZUSS/Ec4faX/9xpDDIW7sSfhM G/PRoCSzxTv7FHgIV7Wo9WrLsxEbNkhgV28QL322qjW+uaS1LqhOmwkh1smyWAyTqssD ZAR4dxpixELpUd2NTUDoMZNbczxrmFJnvjZCe0qY30yXK0KHYIj7o8G3EI5a8cWlEKc5 2qRhiF79GEWzSI96YbpZonf9KiN0x0/Ka3Y3JslCo9QBLnX2iuzYWxjItJ4rGE8uZl// 97Lg== X-Gm-Message-State: AOJu0Yz/iSzJpZxbONKyWz2TxowIfYSMDsryby6DhHhnP3inUEBZFvpO SLq132EgMuRNOLyGP0rU/KJiUXj1DHWt+n7+KkVgK4fBSyxIl1JcvfmlruVnP65VcnI+XMAKggZ skXgn8D+YjiOny4wBLJQwIHAirS4= X-Google-Smtp-Source: AGHT+IGe18EruMLkjQACUiIKFNnJXh9s23nT4tGrKr/3Pqbc3/mi3z32SzMQHrIrmFq5GPqTY3+SE7sWPfHNTP4QLLo= X-Received: by 2002:a19:5f0a:0:b0:52b:c08d:2da4 with SMTP id 2adb3069b0e04-52ccaa33c19mr7262901e87.25.1719062377144; Sat, 22 Jun 2024 06:19:37 -0700 (PDT) MIME-Version: 1.0 References: <20240621035252.742099-1-pan2.li@intel.com> In-Reply-To: From: Richard Biener Date: Sat, 22 Jun 2024 15:19:25 +0200 Message-ID: Subject: Re: [PATCH v1] Ifcvt: Add cond tree reconcile for truncated .SAT_SUB To: "Li, Pan2" Cc: "gcc-patches@gcc.gnu.org" , "juzhe.zhong@rivai.ai" , "kito.cheng@gmail.com" , "jeffreyalaw@gmail.com" , "rdapp.gcc@gmail.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,WEIRD_PORT 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 Fri, Jun 21, 2024 at 4:45=E2=80=AFPM Li, Pan2 wrote: > > Thanks Richard for suggestion, tried the (convert? with below gimple stmt= but got a miss def ice. > To double confirm, the *type_out should be the vector type of lhs, and we= only need to build > one cvt stmt from itype to otype here. Or just return the call directly a= nd set the type_out to the v_otype? > > static gimple * > vect_recog_build_binary_gimple_stmt (vec_info *vinfo, gimple *stmt, > internal_fn fn, tree *type_out, > tree lhs, tree op_0, tree op_1) > { > tree itype =3D TREE_TYPE (op_0); > tree otype =3D TREE_TYPE (lhs); > tree v_itype =3D get_vectype_for_scalar_type (vinfo, itype); > tree v_otype =3D get_vectype_for_scalar_type (vinfo, otype); > > if (v_itype !=3D NULL_TREE && v_otype !=3D NULL_TREE > && direct_internal_fn_supported_p (fn, v_itype, OPTIMIZE_FOR_BOTH)) > { > gcall *call =3D gimple_build_call_internal (fn, 2, op_0, op_1); > tree itype_ssa =3D vect_recog_temp_ssa_var (itype, NULL); > > gimple_call_set_lhs (call, itype_ssa); > gimple_call_set_nothrow (call, /* nothrow_p */ false); > gimple_set_location (call, gimple_location (stmt)); > > *type_out =3D v_otype; > gimple *new_stmt =3D call; > > if (itype !=3D otype) > { > tree otype_ssa =3D vect_recog_temp_ssa_var (otype, NULL); > new_stmt =3D gimple_build_assign (otype_ssa, CONVERT_EXPR, ityp= e_ssa); > } > > return new_stmt; You need to refactor this to add to the stmts pattern def sequence (look for append_pattern_def_seq uses for example) > } > > return NULL; > } > > -----cut the ice--- > > zip.test.c: In function =E2=80=98test=E2=80=99: > zip.test.c:4:6: error: missing definition > 4 | void test (uint16_t *x, unsigned b, unsigned n) > | ^~~~ > for SSA_NAME: patt_40 in statement: > vect_cst__151 =3D [vec_duplicate_expr] patt_40; > during GIMPLE pass: vect > dump file: zip.test.c.180t.vect > zip.test.c:4:6: internal compiler error: verify_ssa failed > 0x1de0860 verify_ssa(bool, bool) > /home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/= tree-ssa.cc:1203 > 0x1919f69 execute_function_todo > /home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/= passes.cc:2096 > 0x1918b46 do_per_function > /home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/= passes.cc:1688 > 0x191a116 execute_todo > > Pan > > > -----Original Message----- > From: Richard Biener > Sent: Friday, June 21, 2024 5:29 PM > To: Li, Pan2 > Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; = jeffreyalaw@gmail.com; rdapp.gcc@gmail.com > Subject: Re: [PATCH v1] Ifcvt: Add cond tree reconcile for truncated .SAT= _SUB > > On Fri, Jun 21, 2024 at 10:50=E2=80=AFAM Li, Pan2 wro= te: > > > > Thanks Richard for comments. > > > > > to match this by changing it to > > > > > /* Unsigned saturation sub, case 2 (branch with ge): > > > SAT_U_SUB =3D X >=3D Y ? X - Y : 0. */ > > > (match (unsigned_integer_sat_sub @0 @1) > > > (cond^ (ge @0 @1) (convert? (minus @0 @1)) integer_zerop) > > > (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type) > > > && types_match (type, @0, @1)))) > > > > Do we need another name for this matching ? Add (convert? here may chan= ge the sematics of .SAT_SUB. > > When we call gimple_unsigned_integer_sat_sub (lhs, ops, NULL), the conv= erted value may be returned different > > to the (minus @0 @1). Please correct me if my understanding is wrong. > > I think gimple_unsigned_integer_sat_sub (lhs, ...) simply matches > (typeof LHS).SAT_SUB (ops[0], ops[1]) now, I don't think it's necessary t= o > handle the case where typef LHS and typeof ops[0] are equal specially? > > > > and when using the gimple_match_* function make sure to consider > > > that the .SAT_SUB (@0, @1) is converted to the type of the SSA name > > > we matched? > > > > This may have problem for vector part I guess, require some additional = change from vectorize_convert when > > I try to do that in previous. Let me double check about it, and keep yo= u posted. > > You are using gimple_unsigned_integer_sat_sub from pattern recognition, t= he > thing to do is simply to add a conversion stmt to the pattern sequence in= case > the types differ? > > But maybe I'm missing something. > > Richard. > > > Pan > > > > -----Original Message----- > > From: Richard Biener > > Sent: Friday, June 21, 2024 3:00 PM > > To: Li, Pan2 > > Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com= ; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com > > Subject: Re: [PATCH v1] Ifcvt: Add cond tree reconcile for truncated .S= AT_SUB > > > > On Fri, Jun 21, 2024 at 5:53=E2=80=AFAM wrote: > > > > > > From: Pan Li > > > > > > The zip benchmark of coremark-pro have one SAT_SUB like pattern but > > > truncated as below: > > > > > > void test (uint16_t *x, unsigned b, unsigned n) > > > { > > > unsigned a =3D 0; > > > register uint16_t *p =3D x; > > > > > > do { > > > a =3D *--p; > > > *p =3D (uint16_t)(a >=3D b ? a - b : 0); // Truncate the result o= f SAT_SUB > > > } while (--n); > > > } > > > > > > It will have gimple after ifcvt pass, it cannot hit any pattern of > > > SAT_SUB and then cannot vectorize to SAT_SUB. > > > > > > _2 =3D a_11 - b_12(D); > > > iftmp.0_13 =3D (short unsigned int) _2; > > > _18 =3D a_11 >=3D b_12(D); > > > iftmp.0_5 =3D _18 ? iftmp.0_13 : 0; > > > > > > This patch would like to do some reconcile for above pattern to match > > > the SAT_SUB pattern. Then the underlying vect pass is able to vector= ize > > > the SAT_SUB. > > > > Hmm. I was thinking of allowing > > > > /* Unsigned saturation sub, case 2 (branch with ge): > > SAT_U_SUB =3D X >=3D Y ? X - Y : 0. */ > > (match (unsigned_integer_sat_sub @0 @1) > > (cond^ (ge @0 @1) (minus @0 @1) integer_zerop) > > (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type) > > && types_match (type, @0, @1)))) > > > > to match this by changing it to > > > > /* Unsigned saturation sub, case 2 (branch with ge): > > SAT_U_SUB =3D X >=3D Y ? X - Y : 0. */ > > (match (unsigned_integer_sat_sub @0 @1) > > (cond^ (ge @0 @1) (convert? (minus @0 @1)) integer_zerop) > > (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type) > > && types_match (type, @0, @1)))) > > > > and when using the gimple_match_* function make sure to consider > > that the .SAT_SUB (@0, @1) is converted to the type of the SSA name > > we matched? > > > > Richard. > > > > > _2 =3D a_11 - b_12(D); > > > _18 =3D a_11 >=3D b_12(D); > > > _pattmp =3D _18 ? _2 : 0; // .SAT_SUB pattern > > > iftmp.0_13 =3D (short unsigned int) _pattmp; > > > iftmp.0_5 =3D iftmp.0_13; > > > > > > The below tests are running for this patch. > > > 1. The rv64gcv fully regression tests. > > > 2. The rv64gcv build with glibc. > > > 3. The x86 bootstrap tests. > > > 4. The x86 fully regression tests. > > > > > > gcc/ChangeLog: > > > > > > * match.pd: Add new match for trunated unsigned sat_sub. > > > * tree-if-conv.cc (gimple_truncated_unsigned_integer_sat_sub)= : > > > New external decl from match.pd. > > > (tree_if_cond_reconcile_unsigned_integer_sat_sub): New func i= mpl > > > to reconcile the truncated sat_sub pattern. > > > (tree_if_cond_reconcile): New func impl to reconcile. > > > (pass_if_conversion::execute): Try to reconcile after ifcvt. > > > > > > Signed-off-by: Pan Li > > > --- > > > gcc/match.pd | 9 +++++ > > > gcc/tree-if-conv.cc | 83 +++++++++++++++++++++++++++++++++++++++++++= ++ > > > 2 files changed, 92 insertions(+) > > > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > > index 3d0689c9312..9617a5f9d5e 100644 > > > --- a/gcc/match.pd > > > +++ b/gcc/match.pd > > > @@ -3210,6 +3210,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > > (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type) > > > && types_match (type, @0, @1)))) > > > > > > +/* Unsigned saturation sub and then truncated, aka: > > > + Truncated =3D X >=3D Y ? (Other Type) (X - Y) : 0. > > > + */ > > > +(match (truncated_unsigned_integer_sat_sub @0 @1) > > > + (cond (ge @0 @1) (convert (minus @0 @1)) integer_zerop) > > > + (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type) > > > + && types_match (@0, @1) > > > + && tree_int_cst_lt (TYPE_SIZE (type), TYPE_SIZE (TREE_TYPE (@0= )))))) > > > + > > > /* x > y && x !=3D XXX_MIN --> x > y > > > x > y && x =3D=3D XXX_MIN --> false . */ > > > (for eqne (eq ne) > > > diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc > > > index 57992b6deca..535743130f2 100644 > > > --- a/gcc/tree-if-conv.cc > > > +++ b/gcc/tree-if-conv.cc > > > @@ -3738,6 +3738,87 @@ bitfields_to_lower_p (class loop *loop, > > > return !reads_to_lower.is_empty () || !writes_to_lower.is_empty ()= ; > > > } > > > > > > +extern bool gimple_truncated_unsigned_integer_sat_sub (tree, tree*, > > > + tree (*)(tree)= ); > > > + > > > +/* > > > + * Try to reconcile the stmt pattern as below to math the SAT_SUB > > > + * in vectorization. If and only if the related internal_fn has > > > + * been implemented already. > > > + * > > > + * The reconcile will insert one new stmt named 'a' in below example= , > > > + * replace the stmt '4' by new added stmt 'b' as well. Then the stm= t > > > + * pattern is able to hit the SAT_SUB pattern in the underlying pass= . > > > + * > > > + * 1. _2 =3D a_11 - b_12(D); > > > + * 2. iftmp.0_13 =3D (short unsigned int) _2; > > > + * 3. _18 =3D a_11 >=3D b_12(D); > > > + * 4. iftmp.0_5 =3D _18 ? iftmp.0_13 : 0; > > > + * =3D=3D> > > > + * 1. _2 =3D a_11 - b_12(D); > > > + * 3. _18 =3D a_11 >=3D b_12(D); > > > + * a. pattmp =3D _18 ? _2 : 0; // New insertion > > > + * 2. iftmp.0_13 =3D (short unsigned int) _pattmp; // Move before > > > + * b. iftmp.0_5 =3D iftmp.0_13; > > > + * =3D=3D Replace =3D=3D> 4. iftmp.0_5 =3D _18 ? iftmp.0_13 : 0; > > > + */ > > > +static void > > > +tree_if_cond_reconcile_unsigned_integer_sat_sub (gimple_stmt_iterato= r *gsi, > > > + gassign *stmt) > > > +{ > > > + tree ops[2]; > > > + tree lhs =3D gimple_assign_lhs (stmt); > > > + bool supported_p =3D direct_internal_fn_supported_p (IFN_SAT_SUB, > > > + TREE_TYPE (lhs), > > > + OPTIMIZE_FOR_BOT= H); > > > + > > > + if (supported_p && gimple_truncated_unsigned_integer_sat_sub (lhs,= ops, NULL)) > > > + { > > > + tree cond =3D gimple_assign_rhs1 (stmt); // aka _18 > > > + tree truncated =3D gimple_assign_rhs2 (stmt); // aka iftmp.0_1= 3 > > > + gimple *stmt_2 =3D SSA_NAME_DEF_STMT (truncated); > > > + tree minus =3D gimple_assign_rhs1 (stmt_2); // aka _2 > > > + tree raw_type =3D TREE_TYPE (minus); > > > + tree zero =3D build_zero_cst (raw_type); > > > + tree tmp =3D make_temp_ssa_name (raw_type, NULL, "sat_sub_tmp"= ); > > > + > > > + /* For stmt 'a' in above example */ > > > + gimple *stmt_a =3D gimple_build_assign (tmp, COND_EXPR, cond, = minus, zero); > > > + gsi_insert_before (gsi, stmt_a, GSI_SAME_STMT); > > > + update_stmt (stmt_a); > > > + > > > + /* For stmt '2' in above example */ > > > + gimple_stmt_iterator stmt_2_gsi =3D gsi_for_stmt (stmt_2); > > > + gsi_move_before (&stmt_2_gsi, gsi, GSI_SAME_STMT); > > > + gimple_assign_set_rhs1 (stmt_2, tmp); > > > + update_stmt (stmt_2); > > > + > > > + /* For stmt 'b' in above example */ > > > + gimple *stmt_b =3D gimple_build_assign (lhs, NOP_EXPR, truncat= ed); > > > + gsi_replace (gsi, stmt_b, /* update_eh_info */ true); > > > + update_stmt (stmt_b); > > > + } > > > +} > > > + > > > +static void > > > +tree_if_cond_reconcile (function *fun) > > > +{ > > > + basic_block bb; > > > + FOR_EACH_BB_FN (bb, fun) > > > + { > > > + gimple_stmt_iterator gsi; > > > + for (gsi =3D gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&g= si)) > > > + { > > > + gimple *stmt =3D gsi_stmt (gsi); > > > + > > > + if (is_gimple_assign (stmt)) > > > + { > > > + gassign *assign =3D dyn_cast (stmt); > > > + tree_if_cond_reconcile_unsigned_integer_sat_sub (&gsi, = assign); > > > + } > > > + } > > > + } > > > +} > > > > > > /* If-convert LOOP when it is legal. For the moment this pass has n= o > > > profitability analysis. Returns non-zero todo flags when somethi= ng > > > @@ -4063,6 +4144,8 @@ pass_if_conversion::execute (function *fun) > > > } > > > } > > > > > > + tree_if_cond_reconcile (fun); > > > + > > > return 0; > > > } > > > > > > -- > > > 2.34.1 > > >