Hi Richard,

Thanks for your comments!

on 2020/6/23 上午3:59, Richard Sandiford wrote:
> "Kewen.Lin" <linkw@linux.ibm.com> writes:
>> @@ -5167,6 +5167,24 @@ mode @var{n}.
>>  
>>  This pattern is not allowed to @code{FAIL}.
>>  
>> +@cindex @code{lenload@var{m}} instruction pattern
>> +@item @samp{lenload@var{m}}
>> +Perform a vector load with length from memory operand 1 of mode @var{m}
>> +into register operand 0.  Length is provided in register operand 2 with
>> +appropriate mode which should afford the maximal required precision of
>> +any available lengths.
> 
> I think we need to say in more detail what “load with length” actually
> means.  How about:
> 
>   Load the number of bytes specified by operand 2 from memory operand 1
>   into register operand 0, setting the other bytes of operand 0 to
>   undefined values.  Operands 0 and 1 have mode @var{m}.  Operand 2 has
>   whichever integer mode the target prefers.
> 

Thanks for nice wordings!  Updated, for "... to undefined values" I changed it
to "... to undefined values or zeros" as Segher's comments to match the behavior
on Power.

> Similarly here:
> 
>   Store the number of bytes specified by operand 2 from nonmemory operand 1
>   into memory operand 0, leaving the other bytes of operand 0 unchanged.
>   Operands 0 and 1 have mode @var{m}.  Operand 2 has whichever integer
>   mode the target prefers.
> 

Done.

>> @@ -2478,7 +2480,7 @@ expand_call_mem_ref (tree type, gcall *stmt, int index)
>>    return fold_build2 (MEM_REF, type, addr, build_int_cst (alias_ptr_type, 0));
>>  }
>>  
>> -/* Expand MASK_LOAD{,_LANES} call STMT using optab OPTAB.  */
>> +/* Expand MASK_LOAD{,_LANES} and LEN_LOAD call STMT using optab OPTAB.  */
> 
> s/and/or/.
> 

Done.

>>  
>>  static void
>>  expand_mask_load_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
> 
> Think it would be worth generalising the name, e.g. to
> expand_partial_load_optab_fn, and adding a #define for
> expand_mask_load_optab_fn before the other two #defines.
> 
> Same comments for stores.
> 

Done.

>> +OPTAB_D (lenload_optab, "lenload$a")
>> +OPTAB_D (lenstore_optab, "lenstore$a")
> 
> Sorry, I should have picked up on this last time, but I think we should
> be consistent about whether there's an underscore after “len” or not.
> I realise this is just replicating what happens for IFN_MASK_LOAD/
> “maskload” and IFN_MASK_STORE/“maskstore”, but it's something I kept
> tripping over when implementing those for SVE.
> 
> Personally I think it is easier to read with the underscore, so this
> would be “len_load_optab” and “len_load$a” (or “len_load_$a”,
> there's no real consistency on that).  Same for stores.
> 

Good point!  I found there are two flavors on the optab naming.

  OPTAB_CD(maskload_optab, "maskload$a$b")
  OPTAB_CD(gather_load_optab, "gather_load$a$b")
  ...
vs. 
  OPTAB_D (vec_realign_load_optab, "vec_realign_load_$a")

Finally I chose two underscores as Segher's comment on readability.

----------------------------

v5:
  - Updated lenload/lenstore optab to len_load/len_store and the docs.
  - Rename expand_mask_{load,store}_optab_fn to expand_partial_{load,store}_optab_fn
  - Added/updated macros for expand_mask_{load,store}_optab_fn
    and expand_len_{load,store}_optab_fn

v4: Update len_load_direct/len_store_direct to align with direct optab.

v3: Get rid of length mode hook.


BR,
Kewen
---
gcc/ChangeLog:

2020-MM-DD  Kewen Lin  <linkw@gcc.gnu.org>

	* doc/md.texi (len_load_@var{m}): Document.
	(len_store_@var{m}): Likewise.
	* internal-fn.c (len_load_direct): New macro.
	(len_store_direct): Likewise.
	(expand_len_load_optab_fn): Likewise.
	(expand_len_store_optab_fn): Likewise.
	(direct_len_load_optab_supported_p): Likewise.
	(direct_len_store_optab_supported_p): Likewise.
	(expand_mask_load_optab_fn): New macro.  Original renamed to ...
	(expand_partial_load_optab_fn): ... here.  Add handlings for
	len_load_optab.
	(expand_mask_store_optab_fn): New macro.  Original renamed to ...
	(expand_partial_store_optab_fn): ... here. Add handlings for
	len_store_optab.
	(internal_load_fn_p): Handle IFN_LEN_LOAD.
	(internal_store_fn_p): Handle IFN_LEN_STORE.
	(internal_fn_stored_value_index): Handle IFN_LEN_STORE.
	* internal-fn.def (LEN_LOAD): New internal function.
	(LEN_STORE): Likewise.
	* optabs.def (len_load_optab, len_store_optab): New optab.