Skip to content
This repository was archived by the owner on Dec 22, 2021. It is now read-only.

Conversation

@tlively
Copy link
Member

@tlively tlively commented Sep 5, 2018

There did not seem to be a reason for i64x2 comparison operations to be omitted, so the PR adds them and adjusts instruction opcodes accordingly.

@AndrewScheidecker
Copy link
Contributor

They were explicitly removed by #22, citing consensus at the May 2017 CG meeting.

(I don't have an opinion here, just stating the history)

@dtig
Copy link
Member

dtig commented Sep 5, 2018

It looks like the poll on the compare operations was ambivalent, and not contentious ("No consensus, but not objection either." ). I think it's reasonable to assume that these can be added back if there was performance, or usability evidence that these belong in the set of operations. Is there a precedence for reserved opcode numbers? If so I think the opcodes can be retained without adding the comparison operations.

@tlively
Copy link
Member Author

tlively commented Sep 13, 2018

Ok, I've changed this PR to add holes in the opcode space where operations we explicitly don't support would logically go to improve our forward compatibility story. These holes are represented by crossed out ops in the binary encoding table.

@dtig dtig requested review from dtig and removed request for dtig September 13, 2018 19:05
@dtig
Copy link
Member

dtig commented Sep 14, 2018

I think it's best to leave reserved opcodes as opcode holes instead of representing them as strike-through ops because there's no consensus on these operations being included. Some of these could be added back, or none at all.

@tlively
Copy link
Member Author

tlively commented Sep 19, 2018

@dtig should be good to go now!

@dtig dtig merged commit c6cc6d4 into master Sep 19, 2018
AndrewScheidecker added a commit to WAVM/Wavix that referenced this pull request Sep 19, 2018
952cb9d7b00 Fix serialization of single-result block types
b5a5b271c69 Add per-config library/binary paths to install Export a .cmake file in the build directory that can be used in FIND_PACKAGE(WAVM...) without installing WAVM
2042bb76344 Make RuntimeData.h compile with sizeof(void*) == 4
13fdd92edd4 Fix a place that used _WIN32 directly instead of USE_WINDOWS_SEH
fcb4adb014a int -> I32
14a9400f733 Replace some uses of _WIN32 with _MSC_VER, and add some comments to Platform/Defines.h
98c7a9ffd03 Fix WAVM_DEBUG definition
a2d6d66cb31 Install binaries to configuration-specific directories on Windows
1b2eef2ea06 Link LLVMJIT with LLVM's RuntimeDyld
a5e0d47bf27 Replace LLVMPreInclude.h and LLVMPostInclude.h with macros to disable warnings using __pragma
472830200de Renumber SIMD operators to match changes in WebAssembly/simd#39
55c26862454 Fix compiling on POSIX with static linking
f9a43fc9b16 Fix WAVM_ENABLE_RUNTIME=NO being used for most Travis targets
ff2d998c692 Fix building with WAVM_ENABLE_RUNTIME=NO on POSIX
77a0471a244 Fix compiling with sanitizers/fuzzers
33b0294b428 Fix OSX compile errors
3a05632efc9 Remove vestigial warning suppression

git-subtree-dir: WAVM
git-subtree-split: 952cb9d7b00f4873e7bed8afb315feb63b122eed
@dtig dtig deleted the v2i64-comparisons branch September 4, 2019 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants