[PATCH] RISC-V: Implement autovec abs, vneg, vnot.

[钟居哲]

>> What about the rest of the changes? It's not all typos but I tried
>> to unify the mask/policy handling a bit.
Oh, I see.  You rename get_prefer into get_preferred.
This makes perfect sense to me.




juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-05-19 20:07
To: 钟居哲; gcc-patches; kito.cheng; palmer; Michael Collison; Jeff Law
CC: rdapp.gcc
Subject: Re: [PATCH] RISC-V: Implement autovec abs, vneg, vnot.
>>> +  TAIL_UNDEFINED = -1,
>>> +  MASK_UNDEFINED = -1,
> Why you add this ?
> 
>>> +  void add_policy_operands (enum tail_policy vta = TAIL_UNDEFINED,
>>> +     enum mask_policy vma = MASK_UNDEFINED)
> No, you should just specify this as TAIL_ANY or MASK_ANY as default value.
 
That's the value I intended for "unspecified" i.e. the caller
didn't specify and then set it to the default.  _ANY can work as
well I guess.
 
> 
>>>const_vlmax_p (machine_mode mode)
>>>{
>>>-  poly_uint64 nuints = GET_MODE_NUNITS (mode);
>>>+  poly_uint64 nunits = GET_MODE_NUNITS (mode);
>>>-  return nuints.is_constant ()
>>>+  return nunits.is_constant ()
>>>     /* The vsetivli can only hold register 0~31.  */
>>>-    ? (IN_RANGE (nuints.to_constant (), 0, 31))
>>>+    ? (IN_RANGE (nunits.to_constant (), 0, 31))
>>>     /* Only allowed in VLS-VLMAX mode.  */
>>>     : false;
>>>}
> Meaningless change ?
 
Typo.
 
> 
>>>    /* For the instruction that doesn't require TA, we still need a default value
>>>          to emit vsetvl. We pick up the default value according to prefer policy. */
>>>    -  return (bool) (get_prefer_tail_policy () & 0x1
>>>    - || (get_prefer_tail_policy () >> 1 & 0x1));
>>>    +  return (bool) (get_preferred_tail_policy () & 0x1
>>>    + || (get_preferred_tail_policy () >> 1 & 0x1));
>>>    }
>>>    /* Get default mask policy.  */
>>>    @@ -576,8 +576,8 @@ get_default_ma ()
>>>    {
>>>       /* For the instruction that doesn't require MA, we still need a default value
>>>          to emit vsetvl. We pick up the default value according to prefer policy. */
>>>    -  return (bool) (get_prefer_mask_policy () & 0x1
>>>    - || (get_prefer_mask_policy () >> 1 & 0x1));
>>>    +  return (bool) (get_preferred_mask_policy () & 0x1
>>>    + || (get_preferred_mask_policy () >> 1 & 0x1));
> Why you change it ?
 
Typo/grammar imho.
 
What about the rest of the changes? It's not all typos but I tried
to unify the mask/policy handling a bit. 
 
> You are using comparison helper which I added one in my downstream 
> when I am working on comparison autovec patterns:
> 
> I think you can normalize my code with yours:
 
I wasn't aware that I'm only using one of several helpers, just refactored
what iss upstream.  Yes your code looks reasonable and it surely works
with the patch without much rework. 
 
> I am almost done all comparison autovec patterns, soon will send them after testing.
 
Good, looking forward to it.
 
Regards
Robin