Categorize ops previously "under consideration" #121
Conversation
| * Int32Clz - count leading zeroes (defined for all values, including 0) | ||
| * Int32Ctz - count trailing zeroes (defined for all values, including 0) | ||
| * Int32Popcnt - count number of ones | ||
| * Int32BSwap - reverse bytes (endian conversion) |
There was a problem hiding this comment.
Could you add the Int64 versions too, since it sounds likely that we'll have 64-bit integer arithmetic in MVP or soon thereafter? Same comment on other integer operations in FutureFeatures.md.
Or instead, we had discussed making the documentation generic for int32/int64 and float32/float64, so that we don't repeat everything twice. Should we do this instead? In another PR?
There was a problem hiding this comment.
I'd prefer making the documentation (mostly) generic for int32/int64 and float32/float64. This wasn't done because WebAssembly/spec#42 (comment). @titzer, can we change your mind :-)?
|
Sorry for initially missing this review! I only just saw it as I was going through the open PR list. |
sunfishcode
force-pushed
the
categorize-ops
branch
2 times, most recently
from
54136a9 to
bc04185
Compare
|
If we can defer the Int32/Int64 refactoring, I believe this update addresses all comments so far. |
| For 16-bit floating-point support, it may make sense to split the feature | ||
| into two parts: support for just converting between 16-bit and 32-bit | ||
| formats possibly folded into load and store operations, and full support | ||
| for actual 16-bit arithmetic. |
|
Mentioned conversion between float16 and float64. |
sunfishcode
force-pushed
the
categorize-ops
branch
from
3dd35c7 to
443de79
Compare
|
Following the same reasoning as in #135, I added an additional patch here which moves clz/ctz/popcount/bswap and trunc/nearestInt into the MVP. |
sunfishcode
force-pushed
the
categorize-ops
branch
2 times, most recently
from
157d0cf to
46d7ca0
Compare
|
Ok, for people who haven't followed the whole sequence here, this PR adds these to MVP.md: Integer: Popcnt, Clz, and Ctz Because they're very useful, quite common in popular hardware, most of them are in JS or are expected to be in the future, and as with Int64, even if JS doesn't have them today, emulating them in the polyfill is not hard and no worse than what compilers would have to do if we don't add them. And, it moves a bunch of stuff out of "under consideration" sections into FutureFeatures.md and adds some new ideas to FutureFeatures.md as well. |
|
sgtm As we said before, we're trying to be minimal with the MVP, but at the coarse feature level, not at the individual op level where the ops are trivial/cheap to implement. Also, that these ops are in multiple popular ISAs speaks strongly for their value; I don't think we have to re-validate this for ourselves. |
| @@ -213,3 +206,96 @@ use cases: | |||
| things possible. Possibly this could involve throwing or possibly by | |||
| resuming execution at the trapping instruction with the execution state | |||
| altered, if there can be a reasonable way to specify how that should work. | |||
|
|
|||
| ## Additional integer operations | |||
|
lgtm after nit. |
|
On Fri, Jun 12, 2015 at 8:59 AM, Dan Gohman [email protected]
Are we talking binary min/max or N-ary min/max?
|
|
Binary min/max only, definitely. I'll add a note about that. |
|
On Fri, Jun 12, 2015 at 10:06 AM, Dan Gohman [email protected]
sgtm
|
sunfishcode
force-pushed
the
categorize-ops
branch
from
46d7ca0 to
683e7c5
Compare
|
lgtm |
ctz/clz/popcount/bswap - not nicely polyfillable (ES6 will have clz, but not all browsers will have that in the timeframe of the polyfill), but quite essential in a modern ISA. These can be done in software and pattern-matched, but except maybe ctz, the patterns are sufficiently gross that I don't expect these will be controversial. rot/min/max - nice to have, but not quite as essential, so put them in FutureFeatures.md.
One of the main uses for these is implementing division-by-constant optimizations, and I think we can call on JITs to do those, so we don't need WebAssembly produers doing them and emitting larger WebAssembly code. For other uses, real 64-bit integers will work fairly well when we get those.
min/max - Promote these to base; they are all trivially polyfillable, and are useful things to have in the base platform besides. nearestInt - This is not trivially polyfillable (JavaScript's Math.round does tie-breaking wrong), so put it in EssentialPostMVPFeatures.md so it can join floor/ceil. trunc - This is not trivially polyfillable (in ES6, but not in all the browsers we need the polyfill to support), so put it in EssentialPostMVPFeatures.md so it can join floor/ceil/nearestInt in providing rouding to integer via all the standard roundings. minNum/maxNum - Put these in FutureFeatures.md for future consideration. They're not trivially polyfillable, and they're not obviously essential at this point, though they do have uses.
Bang is unary ! Neg is unary -
And add a paragraph about 128-bit floating-point support. Also specify that FMA would be IEEE-754 or not available.
This is inspired by the reasoning for adding Int64 to the MVP: the polyfill isn't going to be worse than what compilers would have to lower these things into otherwise, and having them there means a nicer path forward to future versions.
If we don't accept !=, we'll probably want to revisit some of these.
sunfishcode
force-pushed
the
categorize-ops
branch
from
683e7c5 to
b2c9664
Compare
|
Nit addressed. I'm taking the liberty of calling several approving posts above enough of a consensus to merge this. Further discussion is of course welcome. |
Categorize ops previously "under consideration"
This assigns more specific categorizations to several things that were "under consideration".
Summary: