Address comment and fix it in this V2 patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613608.html
juzhe.zh...@rivai.ai
From: Bernhard Reutner-Fischer
Date: 2023-03-09 05:16
To: juzhe.zhong; gcc-patches
CC: kito.cheng; Ju-Zhe Zhong
Subject: Re: [PATCH] RISC-V: Add fault first load C/C++ support
On 7 March 2023 07:21:23 CET, juzhe.zh...@rivai.ai wrote:
>From: Ju-Zhe Zhong <juzhe.zh...@rivai.ai>
>
>+class vleff : public function_base
>+{
>+public:
>+ unsigned int call_properties (const function_instance &) const override
>+ {
>+ return CP_READ_MEMORY | CP_WRITE_CSR;
>+ }
>+
>+ gimple *fold (gimple_folder &f) const override
>+ {
>+ /* fold vleff (const *base, size_t *new_vl, size_t vl)
>+
>+ ====> vleff (const *base, size_t vl)
>+ new_vl = MEM_REF[read_vl ()]. */
>+
>+ auto_vec<tree, 8> vargs;
Where is that magic 8 coming from?
Wouldn't you rather have one temporary to hold this manually CSEd
nargs = gimple_call_num_args (f.call) - 2;
which you would use throughout this function as it does not seem to change?
Would you reserve something based off nargs for the auto_vec above?
If not, please add a comment where the 8 comes from?
thanks,
>+
>+ for (unsigned i = 0; i < gimple_call_num_args (f.call); i++)
>+ {
>+ /* Exclude size_t *new_vl argument. */
>+ if (i == gimple_call_num_args (f.call) - 2)
>+ continue;
>+
>+ vargs.quick_push (gimple_call_arg (f.call, i));
>+ }
>+
>+ gimple *repl = gimple_build_call_vec (gimple_call_fn (f.call), vargs);
>+ gimple_call_set_lhs (repl, f.lhs);
>+
>+ /* Handle size_t *new_vl by read_vl. */
>+ tree new_vl = gimple_call_arg (f.call, gimple_call_num_args (f.call) - 2);
>+ if (integer_zerop (new_vl))
>+ {
>+ /* This case happens when user passes the nullptr to new_vl argument.
>+ In this case, we just need to ignore the new_vl argument and return
>+ vleff instruction directly. */
>+ return repl;
>+ }
>+
>+ tree tmp_var = create_tmp_var (size_type_node, "new_vl");
>+ tree decl = get_read_vl_decl ();
>+ gimple *g = gimple_build_call (decl, 0);
>+ gimple_call_set_lhs (g, tmp_var);
>+ tree indirect
>+ = fold_build2 (MEM_REF, size_type_node,
>+ gimple_call_arg (f.call,
>+ gimple_call_num_args (f.call) - 2),
>+ build_int_cst (build_pointer_type (size_type_node), 0));
>+ gassign *assign = gimple_build_assign (indirect, tmp_var);
>+
>+ gsi_insert_after (f.gsi, assign, GSI_SAME_STMT);
>+ gsi_insert_after (f.gsi, g, GSI_SAME_STMT);
>+ return repl;
>+ }
>+
