Hi WeiWei,

Thanks for reviewing this PR.

===============================================================================

Regarding to possible behaviors on agnostic elements to mask instructions, I
want to ask for you and other's opinion on this proposed PR before sending the
next version.

I understand that there are multiple possibility for agnostic elements
according to v-spec. The main intent of this patch-set tries to add option that
can distinguish between tail policies. Setting agnostic elements to all 1s
makes things simple and allow qemu to express that the element is agnostic.
Therefore I want unify **all** agnostic elements to be set to 1s in this when
this option is enabled.

To avoid affecting the current behavior, the option is default to “disabled".
This option is an extra feature to offer so users that care can enable it on
their will.

===============================================================================

Here are some replies to your review comments.

Under [00/13]
> Another question: when rvv_ta_all_1s for vta  is enabled, How about vma? 
> Is it necessary to set the inactive elements to all 1s?

This PR will add tail agnostic feature. I am planning on adding the mask
policy in another PR to keep the size of change more reasonable for review.

Under [01/13]
> ESZ can be used in the later patches. Maybe it's better to move this 
> patch to  last  and prune redundant DSZ parameter.

ESZ and DSZ are redundant code that aren't cleaned-up in the past developments.
I prefer to clean this up first and add it back incrementally in the following
commit to make the commits more readable. I do agree with you that `ETYPE` is
not a straight-forward naming and I will change them to `ESZ`.

Under [03/13]
> Maybe miss a space here.

Nice catch here, thank you.

Under [04/13]
> ETYPE seems have no other meaning here. Why not use ESZ directly  as 
original code.

Yes I agree with you. I will update it in the next version.

Under [05/13]
> Similar to last patch, can use ESZ directly here.

I will update it in the next version.

Under [06/13]
> Use vlmax here and in the previous patches may not contains all the tail 
> elements:
> "When LMUL < 1, the tail includes the elements past VLMAX that are held 
> in the same vector register"

Nice catch for this. I will cover LMUL < 1 cases for all functions in the next
version.

Under [07/13]
> Why comment 'clear tail element' here?
> "In addition, except for mask load instructions, any element in the tail 
> of a mask result can also be written with the value the
> mask-producing operation would have calculated with vl=VLMAX.
> Furthermore, for mask-logical instructions and vmsbf.m,
> vmsif.m, vmsof.m mask-manipulation instructions, any element in the tail 
> of the result can be written with the value the
> mask-producing operation would have calculated with vl=VLEN, SEW=8, and 
> LMUL=8 (i.e., all bits of the mask register can
> be overwritten)."

I will wait for you and other's reply on my comment on this.

===============================================================================

Thanks again for your time.

Best,

Yueh-Ting (eop) Chen

Reply via email to