|Subject:||Re: [PATCH qemu 00/13] Add tail agnostic behavior for rvv instructions|
|Date:||Mon, 21 Mar 2022 19:33:54 +0800|
|User-agent:||Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0|
Hi WeiWei,Thanks for reviewing this PR.===============================================================================Regarding to possible behaviors on agnostic elements to mask instructions, Iwant to ask for you and other's opinion on this proposed PR before sending thenext version.I understand that there are multiple possibility for agnostic elementsaccording to v-spec. The main intent of this patch-set tries to add option thatcan distinguish between tail policies. Setting agnostic elements to all 1smakes 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 whenthis 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 ontheir 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 maskpolicy 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?
OK. It's also acceptable to me.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 followingcommit to make the commits more readable. I do agree with you that `ETYPE` isnot 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 asoriginal 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 nextversion.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.
===============================================================================Thanks again for your time.Best,Yueh-Ting (eop) Chen
|[Prev in Thread]||Current Thread||[Next in Thread]|