Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should reduction have a dest/merge operand? #25

Closed
rofirrim opened this issue Jun 18, 2020 · 5 comments
Closed

Should reduction have a dest/merge operand? #25

rofirrim opened this issue Jun 18, 2020 · 5 comments

Comments

@rofirrim
Copy link
Collaborator

Operations that generate a "scalar vector" (e.g. vmv.s.x, vfmv.s.f) have a dest operand of the kind of the destination so the original value, except element 0, can be preserved.

For instance

vint32m1_t vmv_s_x_i32m1 (vint32m1_t dst, int32_t src);

Reductions also generate "scalar vectors" but don't seem to have the same treatment.

vint32m1_t vredmax_vs_i32m2_i32m1 (vint32m2_t vector, vint32m1_t scalar);

Do we want to have a dest operand in this case? Like this:

vint32m1_t vredmax_vs_i32m2_i32m1 (vin32m1_t dest, vint32m2_t vector, vint32m1_t scalar);

I don't think it is super fundamental but maybe someone wants to preserve the other elements for some reason?

@kito-cheng
Copy link
Collaborator

vint32m1_t vredmax_vs_i32m2_i32m1 (vin32m1_t dest, vint32m2_t vector, vint32m1_t scalar);

This kind of interface, compiler need merge dest and scalar first, which might emit 1~2 extra instructions, those instructions might able to optimize out later, but it little violate current 1-1 mapping design principle.

And user can use vmv_s_x + vredmax_vs to get same semantics if they want, so I would prefer keep current interfcae.

@rofirrim
Copy link
Collaborator Author

Hi @kito-cheng, thanks for the comments.

I didn't expect scalar be merged into dest. Given that particular instruction

vredmax.vs  vd, vs2, vs1, vm   # vd[0] =  max( vs1[0] , vs2[*] )

The idea was to allow a way to control the rest of elements of vd that are not updated (vd[1:VLMAX-1]). We can achieve this if dest is stored in vd already, without extra instructions, right? (vector would be in vs2 and scalar in vs1)

Perhaps I'm missing something obvious here.

@kito-cheng
Copy link
Collaborator

kito-cheng commented Jun 19, 2020

@rofirrim I checked the v isa spec again, it's my mistake, and sounds like your proposal made the interface more consistent.

@zakk0610
Copy link
Collaborator

+1, I think it's a good idea for consistent interface.

@Hsiangkai
Copy link
Collaborator

I also agree with @rofirrim's proposal. If there is no objection, I will update the document for reduction operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants