Re: [PATCH qemu 00/13] Add tail agnostic behavior for rvv instructions

[Weiwei Li]

在 2022/3/21 下午3:50, eop Chen 写道:

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.

Yeah, It's truly another question. I just wonder about whether they should take the same agnostic policy,

or can take different policies?

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`.

    OK. It's also acceptable to me.

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.

I'm also not sure about this. In my personal opinion:

The policy that overwrite tail element with 1s can also be used here (just the elements become 1 bit) as you

have done. However the comment 'clear tail element'  seems not suitable.

The above two policies can be other choices that enabled by other options.

Regards,

Weiwei Li

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

Thanks again for your time.

Best,

Yueh-Ting (eop) Chen