Re: Re: [PATCH] RISC-V: Add fault first load C/C++ support

[<juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 2660 bytes --]

Address comment and fix it in this V2 patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613608.html



juzhe.zhong@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.zhong@rivai.ai wrote:
>From: Ju-Zhe Zhong <juzhe.zhong@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;
>+  }
>+