>> 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.zh...@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
