From: "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>
To: kito.cheng <kito.cheng@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
palmer <palmer@dabbelt.com>,
jeffreyalaw <jeffreyalaw@gmail.com>
Subject: Re: Re: [PATCH V5] RISC-V: Using merge approach to optimize repeating sequence in vec_init
Date: Wed, 17 May 2023 11:35:26 +0800 [thread overview]
Message-ID: <33BC898DEE8BE5E7+2023051711352621895279@rivai.ai> (raw)
In-Reply-To: <CA+yXCZAu19xVve9Xbh18WFvcC4LBHWGcCJ=cJJRTZcij9zYxFg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6439 bytes --]
>> 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++)` ?
No, sizeof (uint64_t) means uint64_t mask = 0;
>> Do you mind give more comment about this? what it checked and what it did?
The reason we use known_gt (GET_MODE_SIZE (dup_mode), BYTES_PER_RISCV_VECTOR)
since we want are using vector integer mode to generate the mask for example
we generate 0b01010101010101.... mask, we should use a scalar register holding value = 0b010101010...
Then vmv.v.x into a vector,then this vector will be used as a mask.
>> 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?
Since we can't vector floting-point instruction to generate a mask.
>> nit: builder.inner_mode () rather than GET_MODE_INNER (dup_mode)?
They are the same. I can change it using GET_MODE_INNER
>> And I would like have more commnet to explain why we need force_reg here.
Since it will creat ICE.
juzhe.zhong@rivai.ai
From: Kito Cheng
Date: 2023-05-17 11:21
To: juzhe.zhong
CC: gcc-patches; palmer; jeffreyalaw
Subject: Re: [PATCH V5] RISC-V: Using merge approach to optimize repeating sequence in vec_init
> +
> +/* 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.
next prev parent reply other threads:[~2023-05-17 3:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-13 0:20 juzhe.zhong
2023-05-14 22:44 ` 钟居哲
2023-05-17 3:21 ` Kito Cheng
2023-05-17 3:35 ` juzhe.zhong [this message]
2023-05-17 3:52 ` Kito Cheng
2023-05-25 3:12 ` Li, Pan2
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=33BC898DEE8BE5E7+2023051711352621895279@rivai.ai \
--to=juzhe.zhong@rivai.ai \
--cc=gcc-patches@gcc.gnu.org \
--cc=jeffreyalaw@gmail.com \
--cc=kito.cheng@gmail.com \
--cc=palmer@dabbelt.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).