Re: RE: [PATCH v1] RISC-V: Implement RESOLVE_OVERLOADED_BUILTIN for RVV intrinsic

[钟居哲 <juzhe.zhong@rivai.ai>]

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

function_instance
get_read_vl_instance (void)
{
  return function_instance ("read_vl", bases::read_vl, shapes::read_vl,
          none_ops[0], PRED_TYPE_none, &p_none_void_ops);
}

tree
get_read_vl_decl (void)
{
  function_instance instance = get_read_vl_instance ();
  hashval_t hash = instance.hash ();
  registered_function *rfn = function_table->find_with_hash (instance, hash);
  gcc_assert (rfn);
  return rfn->decl;
}

You should reference it. I don't see why it's hard for use to construct instance first, then use that instance hash to get the decl.


juzhe.zhong@rivai.ai
 
From: Li, Pan2
Date: 2023-09-11 20:26
To: juzhe.zhong
CC: kito.cheng; gcc-patches; Wang, Yanzhang
Subject: RE: [PATCH v1] RISC-V: Implement RESOLVE_OVERLOADED_BUILTIN for RVV intrinsic
> No. You must construct instance. 'strcmp' is very ugly.
 
Strcmp here is defensive code here for early exit if not found (can be removed for correctness), which is not required to find the right declaration.
 
Pan
 
From: juzhe.zhong <juzhe.zhong@rivai.ai> 
Sent: Monday, September 11, 2023 8:20 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: kito.cheng <kito.cheng@gmail.com>; gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v1] RISC-V: Implement RESOLVE_OVERLOADED_BUILTIN for RVV intrinsic
 
No. You must construct instance. 'strcmp' is very ugly.
---- Replied Message ----
From
Li, Pan2<pan2.li@intel.com>
Date
09/11/2023 20:09
To
juzhe.zhong@rivai.ai<juzhe.zhong@rivai.ai>,
kito.cheng<kito.cheng@gmail.com>
Cc
gcc-patches<gcc-patches@gcc.gnu.org>,
Wang, Yanzhang<yanzhang.wang@intel.com>
Subject
RE: Re: [PATCH v1] RISC-V: Implement RESOLVE_OVERLOADED_BUILTIN for RVV intrinsic
> -    if (overloaded_p && instance.pred == PRED_TYPE_m)
> +    if (overloaded_p)
 
Thanks for pointing this out, my misunderstanding for policy function result in this change as mistake, will send V2 for this.
 
> Plz change it into : Actually, it is not easy to convert to this approach as aarch64 has different implementation of types information.Like type_suffix_info (aarch64 loop type suffix to get the arglist type in infer_vector_or_tuple_type) etc.Thus, it is not easy to construct rvv_type_info, predication_type_index and rvv_op_info from arglist, these are requiredby function_instance when constructing. Pan
 
From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai> 
Sent: Monday, September 11, 2023 5:13 PM
To: kito.cheng <kito.cheng@gmail.com>
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: Re: [PATCH v1] RISC-V: Implement RESOLVE_OVERLOADED_BUILTIN for RVV intrinsic
 
>> Just make sure it's the right change?
It seem incorrect to me.
 
More comments (I just reviewed again):
 
+tree+function_resolver::lookup ()+{+  unsigned int code_limit = vec_safe_length (registered_functions);++  for (unsigned code = get_sub_code () + 1; code < code_limit; code++)+    {+      registered_function *rfun = (*registered_functions)[code];+      function_instance instance = rfun->instance;++      if (strcmp (base_name, instance.base_name) != 0)+       break;++      if (rfun->overloaded_p)+       continue;++      unsigned k;+      const rvv_arg_type_info *args = instance.op_info->args;++      for (k = 0; args[k].base_type != NUM_BASE_TYPES; k++)+       {+         if (k >= m_arglist.length ())+           break;++         if (TYPE_MODE (instance.get_arg_type (k))+           != TYPE_MODE (TREE_TYPE (m_arglist[k])))+           break;+       }++       if (args[k].base_type == NUM_BASE_TYPES)+         return rfun->decl;+    }++  return NULL_TREE;+} Plz change it into : 
/* Silently check whether there is an instance of the function with the
   mode suffix given by MODE and the type suffixes given by TYPE0 and TYPE1.
   Return its function decl if so, otherwise return null.  */
tree
function_resolver::lookup_form (mode_suffix_index mode,
        type_suffix_index type0,
        type_suffix_index type1)
{
  type_suffix_pair types = { type0, type1 };
  function_instance instance (base_name, base, shape, mode, types, pred);
  registered_function *rfn
    = function_table->find_with_hash (instance, instance.hash ());
  return rfn ? rfn->decl : NULL_TREE;
}


juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-09-11 17:04
To: juzhe.zhong@rivai.ai
CC: pan2.li; gcc-patches; yanzhang.wang
Subject: Re: [PATCH v1] RISC-V: Implement RESOLVE_OVERLOADED_BUILTIN for RVV intrinsic
> @@ -545,7 +563,7 @@ struct move_def : public build_base
>      /* According to rvv-intrinsic-doc, it does not add "_m" suffix
>         for vop_m C++ overloaded API.  */
> -    if (overloaded_p && instance.pred == PRED_TYPE_m)
> +    if (overloaded_p)
 
Just make sure it's the right change?
 
>        return b.finish_name ();
>      b.append_name (predication_suffixes[instance.pred]);
>      return b.finish_name ();