From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x933.google.com (mail-ua1-x933.google.com [IPv6:2607:f8b0:4864:20::933]) by sourceware.org (Postfix) with ESMTPS id A140B3858D1E for ; Wed, 17 May 2023 03:21:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A140B3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ua1-x933.google.com with SMTP id a1e0cc1a2514c-77d508ba6faso130739241.2 for ; Tue, 16 May 2023 20:21:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684293677; x=1686885677; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=mlb1ETG+Lkfl3Z3kT0Tv07X91tGMQQGnhBs0idA0qLM=; b=khhzbK4Cx9csRqN0PkevtYdKY7lmH47vMLE3/gFBXEyoHH+BmeJVf+t5gFm2Fv22tr rCxckKIo5sBdvsYPR3jlMHbZib0s+DBGEDTYqDYahQVUBptlQT0wQo9/NFA0QXFv3oF5 21ZqgjPCg/cogB6Vz+jPWXxn711In40PYXW5EGH96dCloGeyjpPvLvLzzxAjhdo3ysYw afk2Z41N6//52cFNyufDwu6dukw3dN5mINPG33xhpGdSHbGGVmp8jylZmKKFiteMNjW1 os8p2hkTtTsIn5YIDuHWzMwRTyDP8dlEH81it5nkeCiWpMqG6ehCgzkqRhka9dii271u IBrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684293677; x=1686885677; h=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=mlb1ETG+Lkfl3Z3kT0Tv07X91tGMQQGnhBs0idA0qLM=; b=A9hFrTMTBHGxUXFscd3W0V/x+LEHLZppDyfavyNCyT/Q7a5O7dRyBRW9kj21cOthWv j7Z3zLTxpxeb+e7MGuokhJurUtneorFz/0qCkSc/OVdGBU1pMzOoaNIpam6F7qam6+BG YuFhgaLkPcT+G/j/TiSEq6zZ8Uj4oC60i5Yr7hId/nb21YcN9LLYcwnlR2s3/Dkzuwd0 B/89GnqOIWZi94lCA7LJ4TKKlHQ5TtuNtNhDdqVHi3Dzpg0+D+MFfQSk6MFifExIMsRJ LbXWQAiMfpPdlXZCMbzrY13gY8977tNG3vya/c+Rdry3go2Oe2wLVMFUrg0myKN0aT5t tl3Q== X-Gm-Message-State: AC+VfDx5ZNZ2bzqt4kWugrMVI3H9VCcwAaOAr4ApDhl/BTSXzTFSk0qf YBNCOSQM3NCSBSHDBA7xabJ+DHxeYNFLBh+LVTU= X-Google-Smtp-Source: ACHHUZ5hIdG++C069Jt2fMkTrO0J6ZsLCg6zNiUUDZtY7OpVPj6/7AGs6WCR1P2aujDST8eBC533uRnoDgJabHCFiTc= X-Received: by 2002:a67:f5c1:0:b0:436:20e7:e6ea with SMTP id t1-20020a67f5c1000000b0043620e7e6eamr11602249vso.16.1684293676611; Tue, 16 May 2023 20:21:16 -0700 (PDT) MIME-Version: 1.0 References: <20230513002026.321317-1-juzhe.zhong@rivai.ai> In-Reply-To: <20230513002026.321317-1-juzhe.zhong@rivai.ai> From: Kito Cheng Date: Wed, 17 May 2023 11:21:05 +0800 Message-ID: Subject: Re: [PATCH V5] RISC-V: Using merge approach to optimize repeating sequence in vec_init To: juzhe.zhong@rivai.ai Cc: gcc-patches@gcc.gnu.org, palmer@dabbelt.com, jeffreyalaw@gmail.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,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: > + > +/* Get the mask for merge approach. > + > + Consider such following case: > + {a, b, a, b, a, b, a, b, a, b, a, b, a, b, a, b} > + To merge "a", the mask should be 1010.... > + To merge "b", the mask should be 0101.... > +*/ > +rtx > +rvv_builder::get_merge_mask_bitfield (unsigned int index) const > +{ > + uint64_t base_mask = (1ULL << index); > + uint64_t mask = 0; > + for (unsigned int i = 0; i < (sizeof (uint64_t) * 8 / npatterns ()); i++) > + mask |= base_mask << (i * npatterns ()); > + return gen_int_mode (mask, inner_int_mode ()); Does it means we assume inner_int_mode is DImode? (because sizeof (uint64_t)) or it should be something like `for (unsigned int i = 0; i < (GET_MODE_SIZE(inner_int_mode ()) * 8 / npatterns ()); i++)` ? > +} > + > /* Subroutine of riscv_vector_expand_vector_init. > Works as follows: > (a) Initialize TARGET by broadcasting element NELTS_REQD - 1 of BUILDER. > @@ -1226,6 +1307,107 @@ expand_vector_init_insert_elems (rtx target, const rvv_builder &builder, > } > } > > +/* Emit vmv.s.x instruction. */ > + > +static void > +emit_scalar_move_op (rtx dest, rtx src, machine_mode mask_mode) > +{ > + insn_expander<8> e; > + machine_mode mode = GET_MODE (dest); > + rtx scalar_move_mask = gen_scalar_move_mask (mask_mode); > + e.set_dest_and_mask (scalar_move_mask, dest, mask_mode); > + e.add_input_operand (src, GET_MODE_INNER (mode)); > + e.set_len_and_policy (const1_rtx, false); > + e.expand (code_for_pred_broadcast (mode), false); > +} > + > +/* Emit merge instruction. */ > + > +static void > +emit_merge_op (rtx dest, rtx src1, rtx src2, rtx mask) > +{ > + insn_expander<8> e; > + machine_mode mode = GET_MODE (dest); > + e.set_dest_merge (dest); > + e.add_input_operand (src1, mode); > + if (VECTOR_MODE_P (GET_MODE (src2))) > + e.add_input_operand (src2, mode); > + else > + e.add_input_operand (src2, GET_MODE_INNER (mode)); > + > + e.add_input_operand (mask, GET_MODE (mask)); > + e.set_len_and_policy (NULL_RTX, true, true, false); > + if (VECTOR_MODE_P (GET_MODE (src2))) > + e.expand (code_for_pred_merge (mode), false); > + else > + e.expand (code_for_pred_merge_scalar (mode), false); > +} > + > +/* Use merge approach to initialize the vector with repeating sequence. > + v = {a, b, a, b, a, b, a, b}. > + > + v = broadcast (a). > + mask = 0b01010101.... > + v = merge (v, b, mask) > +*/ > +static void > +expand_vector_init_merge_repeating_sequence (rtx target, > + const rvv_builder &builder) > +{ > + machine_mode mask_mode = get_mask_mode (builder.mode ()).require (); > + machine_mode dup_mode = builder.mode (); > + if (known_gt (GET_MODE_SIZE (dup_mode), BYTES_PER_RISCV_VECTOR)) > + { > + poly_uint64 nunits > + = exact_div (BYTES_PER_RISCV_VECTOR, builder.inner_units ()); > + dup_mode = get_vector_mode (builder.inner_int_mode (), nunits).require (); > + } Do you mind give more comment about this? what it checked and what it did? > + else > + { > + if (FLOAT_MODE_P (dup_mode)) > + { > + poly_uint64 nunits = GET_MODE_NUNITS (dup_mode); > + dup_mode > + = get_vector_mode (builder.inner_int_mode (), nunits).require (); > + } Why this only hide in else? I guess I have this question is because I don't fully understand the logic of the if condition? > + } > + > + machine_mode dup_mask_mode = get_mask_mode (dup_mode).require (); > + > + /* Step 1: Broadcast the 1st-pattern. */ > + emit_len_op (code_for_pred_broadcast (builder.mode ()), target, > + force_reg (builder.inner_mode (), builder.elt (0)), NULL_RTX, > + mask_mode); > + > + /* Step 2: Merge each non 1st pattern. */ > + for (unsigned int i = 1; i < builder.npatterns (); i++) > + { > + /* Step 2-1: Generate mask register v0 for each merge. */ > + rtx mask_bitfield = builder.get_merge_mask_bitfield (i); > + rtx mask = gen_reg_rtx (mask_mode); > + rtx dup = gen_reg_rtx (dup_mode); > + if (builder.inner_size () >= builder.full_nelts ().to_constant ()) > + { > + /* Use vmv.s.x. */ > + emit_scalar_move_op (dup, mask_bitfield, dup_mask_mode); > + } > + else > + { > + /* Use vmv.v.x. */ > + unsigned int mask_num = CEIL (builder.full_nelts ().to_constant (), > + builder.inner_size ()); > + rtx vl = gen_int_mode (mask_num, Pmode); > + emit_len_op (code_for_pred_broadcast (dup_mode), dup, > + force_reg (GET_MODE_INNER (dup_mode), mask_bitfield), vl, nit: builder.inner_mode () rather than GET_MODE_INNER (dup_mode)? And I would like have more commnet to explain why we need force_reg here. I guess it's corresponding to FLOAT_MODE_P, but it's not easy to understand at frist moment without comment.