I think it's reasonable to define the count as unsigned too. This looks good to me.

On the issue @titzer didn't oppose adding rotate, but suggested that we may still want to keep it post-MVP. Maybe to quantify the "cost" of rotate it would be useful to do a PR to https://github.com/WebAssembly/v8-native-prototype and add the operation there? If it's truly trivial to add to V8 then I think we'd have a pretty good rationale to add it to MVP from at least one engine's point of view.

WDYT @titzer?

What's the process for moving FutureFeatures into the MVP? At some point we
have to cut off the feature set for MVP and FutureFeatures was one
mechanism we had to do that. I'm not particularly concerned about rotates,
but are they MVP-urgent, balanced against their cost?

On Tue, Nov 17, 2015 at 6:38 AM, JF Bastien notifications@github.com
wrote:

On the issue @titzer https://github.com/titzer didn't oppose adding
rotate, but suggested that we may still want to keep it post-MVP. Maybe to
quantify the "cost" of rotate it would be useful to do a PR to
https://github.com/WebAssembly/v8-native-prototype and add the operation
there? If it's truly trivial to add to V8 then I think we'd have a pretty
good rationale to add it to MVP from at least one engine's point of view.

WDYT @titzer https://github.com/titzer?

—
Reply to this email directly or view it on GitHub
#462 (comment).

One of our repeatedly stated design goals is that we want it to be very fast and easy to iterate on the spec by adding new features. Given this, it seems like rotates would need to be extremely high-priority for us to pull them in the MVP - they're a small, independent feature that could easily be specced and shipped at any point after launch.

The small, independent nature of this feature is what makes it low cost to include in the MVP. I wouldn't describe them as urgent, but useful and not complicated.

Given the nature of this project it is difficult to quantify the cost/benefit of most instructions.
Pattern recognition by an engine is often feasible, which would argue for keeping the ISA small.
But why not take advantage of C/C++ compilers' ability to do the recognition? This would lead to smaller wasm, and be one less feature that compilers have to worry about versioning later.

It's hard to imagine the implementation cost for rotates being any more expensive than popcnt/clz/ctz, or the recently added select operator. How is rotate different from these instructions?

Rotates by themselves are not critical for the MVP. But in general the richer we can make the MVP, the more likely it is to be adopted.

Pattern recognition by an engine is often feasible, which would argue for keeping the ISA small.
But why not take advantage of C/C++ compilers' ability to do the recognition? This would lead to smaller wasm, and be one less feature that compilers have to worry about versioning later.

Features aren't versioned, they're feature-detected. And part of the feature detection design assumes a way to polyfill some subset of unsupported instructions. Any 'rotate' instruction in the opcode table at the top of a module could also include a simple wasm polyfill that implements the rotate.

This isn't a matter of 'rotate is hard' or 'rotate is easier than popcnt', it's more the fundamental question: When are we going to stop pulling features into the MVP? How do we want to draw the line? Arguably, anything nonessential can be bumped until shortly after the MVP.

in general the richer we can make the MVP, the more likely it is to be adopted.

A MVP need only meet the minimum threshold to be viable. Being adopted everywhere is something you optimize for once you've got a working product out the door.

My opinion: this is small, simple, and useful. Having a feature test for it forever seems to outweigh the saving of putting it in FutureFeatures. I suggest validating this assumption by implementing it in v8-proto before moving to MVP.

This is a very ad-hoc process, but I don't think we need to overthink it either :-)

No strong opinion on rotate, but just wanted to point out that features missing from the MVP won't have to be feature-tested forever just like people don't feature test Web APIs forever: at some point a feature reaches ubiquity and can just be assumed.

Per the suggestion from @jfbastien the rotates were prototyped in https://github.com/weilianglin/v8-native-prototype/tree/rotate

The changes were straightforward. The line count is misleading as insertion of opcode numbers resulted in shifting many others by 1.

I believe this demonstrates the low effort/cost of adding this support,
and respectfully request approval of this PR.

The spec PR 149 was also updated to remove bswap, and rename the rotate instructions to rotl/rotr.

Ping; this PR adds the rotate instructions, which were received favorably in offline discussions.

lgtm

@mbodart once had a patch to add these to V8. If they can forward port it to V8 today, I'll land that too :-)

Merging, since we seem to have consensus on these.

@titzer sorry for missing it. I will submit a patch to V8 today.