Skip to content

Fix select#187

Merged
jfbastien merged 1 commit intomasterfrom
select
Feb 5, 2016
Merged

Fix select#187
jfbastien merged 1 commit intomasterfrom
select

Conversation

@jfbastien
Copy link
Member

The ordering changed in: WebAssembly/spec#221
Which changed the spec tests, breaking sexpr-wasm because it pulls in the spec tests. This was then fixed:
WebAssembly/wabt@23dc368
Which in turn breaks when binaryen feeds sexpr-wasm .wast files with the old select operand ordering.

Note that this PR has new failures when running the torture tests in binaryen-shell: the order of evaluation is correct in binaryen-shell but isn't emitted properly by LLVM in the .s files. This will require another patch to fix LLVM.

The ordering changed in: WebAssembly/spec#221
Which changed the spec tests, breaking sexpr-wasm because it pulls in the spec tests. This was then fixed:
WebAssembly/wabt@23dc368
Which in turn breaks when binaryen feeds sexpr-wasm .wast files with the old select operand ordering.

Note that this PR has new failures when running the torture tests in binaryen-shell: the order of evaluation is correct in binaryen-shell but isn't emitted properly by LLVM in the .s files. This will require another patch to fix LLVM.
@jfbastien
Copy link
Member Author

FYI @sunfishcode @binji @kripken @titzer

It would be good to stage these changes next time so different tools don't break (e.g. @titzer is fixing V8 separately). We'll have more such changes as we bring things up and we'll need to have all patches ready before landing them, or at least be ready for the breakage.

jfbastien added a commit that referenced this pull request Feb 5, 2016
@jfbastien jfbastien merged commit 9cfdb12 into master Feb 5, 2016
@jfbastien jfbastien deleted the select branch February 5, 2016 13:14
@sunfishcode
Copy link
Member

@jfbastien I thought the lkgr tracking meant the system could tolerate temporary breakage. Is this not the case?

@jfbastien
Copy link
Member Author

Kind of, it can be improved but right now lkgr stays at the last time all the tip-of-tree components worked instead of individually testing them and then figuring out an lkgr from the ones that work.

In this case it wouldn't have helped though: different components pull the spec tests, so they break when they update the tests. I think in cases like this (or of the binary format, when we get there) we'll want to synchronize changes.

It's not a big issues, we're just churning stuff and figuring out what the dependencies are and how to do things. LMK if there's anything you think could improve here. It's not perfect but I'm hoping it stays low-process and low-pain!

@sunfishcode
Copy link
Member

Would tracking lkgr for the tests as well fix this?

@jfbastien
Copy link
Member Author

Yeah that could help. We'd need to officially move them to their own repo. Let chat next week when I'm back in the US?

@kripken
Copy link
Member

kripken commented Feb 5, 2016

Looks like this duplicates some of #184 which was filed earlier?

@kripken kripken mentioned this pull request Feb 5, 2016
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

Successfully merging this pull request may close these issues.

3 participants