From 7459e5af01fbe3a8e75e73783794b4cdffda34e9 Mon Sep 17 00:00:00 2001
From: JF Bastien <jfb@chromium.org>
Date: Fri, 5 Feb 2016 05:11:50 -0800
Subject: Fix select

The ordering changed in: https://github.com/WebAssembly/spec/pull/221
Which changed the spec tests, breaking sexpr-wasm because it pulls in the spec tests. This was then fixed:
https://github.com/WebAssembly/sexpr-wasm-prototype/commit/23dc368148fc7827a603e3853f5a40287eb9effe
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.
---
 src/asm2wasm.h         |  2 +-
 src/s2wasm.h           |  3 ++-
 src/wasm-interpreter.h |  6 +++---
 src/wasm-s-parser.h    |  6 +++---
 src/wasm.h             |  6 +++---
 src/wasm2asm.h         | 22 +++++++++++-----------
 6 files changed, 23 insertions(+), 22 deletions(-)

(limited to 'src')

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index d56bf0cce..22db2636a 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -1109,9 +1109,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
             flip->right = get();
             flip->type = i32;
             auto select = allocator.alloc<Select>();
-            select->condition = isNegative;
             select->ifTrue = flip;
             select->ifFalse = get();
+            select->condition = isNegative;
             select->type = i32;
             block->list.push_back(select);
             block->type = i32;
diff --git a/src/s2wasm.h b/src/s2wasm.h
index 275cf6a9c..f800e193b 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -688,9 +688,10 @@ class S2WasmBuilder {
       skipComma();
       auto curr = allocator.alloc<Select>();
       auto inputs = getInputs(3);
-      curr->condition = inputs[0];
       curr->ifTrue = inputs[1];
       curr->ifFalse = inputs[2];
+      curr->condition = inputs[0];
+      assert(curr->condition->type == i32);
       curr->type = type;
       setOutput(curr, assign);
     };
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index 022dd04bc..3f9427ed7 100644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -620,13 +620,13 @@ private:
       }
       Flow visitSelect(Select *curr) {
         NOTE_ENTER("Select");
-        Flow condition = visit(curr->condition);
-        if (condition.breaking()) return condition;
-        NOTE_EVAL1(condition.value);
         Flow ifTrue = visit(curr->ifTrue);
         if (ifTrue.breaking()) return ifTrue;
         Flow ifFalse = visit(curr->ifFalse);
         if (ifFalse.breaking()) return ifFalse;
+        Flow condition = visit(curr->condition);
+        if (condition.breaking()) return condition;
+        NOTE_EVAL1(condition.value);
         return condition.value.geti32() ? ifTrue : ifFalse; // ;-)
       }
       Flow visitReturn(Return *curr) {
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index e2a880e51..64dc6a454 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -633,9 +633,9 @@ private:
 
   Expression* makeSelect(Element& s, WasmType type) {
     auto ret = allocator.alloc<Select>();
-    ret->condition = parseExpression(s[1]);
-    ret->ifTrue = parseExpression(s[2]);
-    ret->ifFalse = parseExpression(s[3]);
+    ret->ifTrue = parseExpression(s[1]);
+    ret->ifFalse = parseExpression(s[2]);
+    ret->condition = parseExpression(s[3]);
     ret->type = type;
     return ret;
   }
diff --git a/src/wasm.h b/src/wasm.h
index b9d0d6822..893c777c4 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -911,15 +911,15 @@ class Select : public Expression {
 public:
   Select() : Expression(SelectId) {}
 
-  Expression *condition, *ifTrue, *ifFalse;
+  Expression *ifTrue, *ifFalse, *condition;
 
   std::ostream& doPrint(std::ostream &o, unsigned indent) {
     o << '(';
     prepareColor(o) << printWasmType(type) << ".select";
     incIndent(o, indent);
-    printFullLine(o, indent, condition);
     printFullLine(o, indent, ifTrue);
     printFullLine(o, indent, ifFalse);
+    printFullLine(o, indent, condition);
     return decIndent(o, indent);
   }
 
@@ -1447,9 +1447,9 @@ struct ChildWalker : public WasmWalkerBase<ChildWalker<ParentType>> {
     parent.walk(curr->right);
   }
   void visitSelect(Select *curr) {
-    parent.walk(curr->condition);
     parent.walk(curr->ifTrue);
     parent.walk(curr->ifFalse);
+    parent.walk(curr->condition);
   }
   void visitReturn(Return *curr) {
     parent.walk(curr->value);
diff --git a/src/wasm2asm.h b/src/wasm2asm.h
index c87e81eec..d8ac6376e 100644
--- a/src/wasm2asm.h
+++ b/src/wasm2asm.h
@@ -463,7 +463,7 @@ void Wasm2AsmBuilder::scanFunctionBody(Expression* curr) {
       }
     }
     void visitSelect(Select *curr) {
-      if (parent->isStatement(curr->condition) || parent->isStatement(curr->ifTrue) || parent->isStatement(curr->ifFalse)) {
+      if (parent->isStatement(curr->ifTrue) || parent->isStatement(curr->ifFalse) || parent->isStatement(curr->condition)) {
         parent->setStatement(curr);
       }
     }
@@ -1057,32 +1057,32 @@ Ref Wasm2AsmBuilder::processFunctionBody(Expression* curr, IString result) {
     }
     Ref visitSelect(Select *curr) {
       if (isStatement(curr)) {
-        ScopedTemp tempCondition(i32, parent);
-        GetLocal fakeCondition;
-        fakeCondition.name = tempCondition.getName();
         ScopedTemp tempIfTrue(curr->ifTrue->type, parent);
         GetLocal fakeLocalIfTrue;
         fakeLocalIfTrue.name = tempIfTrue.getName();
         ScopedTemp tempIfFalse(curr->ifFalse->type, parent);
         GetLocal fakeLocalIfFalse;
         fakeLocalIfFalse.name = tempIfFalse.getName();
+        ScopedTemp tempCondition(i32, parent);
+        GetLocal fakeCondition;
+        fakeCondition.name = tempCondition.getName();
         Select fakeSelect = *curr;
-        fakeSelect.condition = &fakeCondition;
         fakeSelect.ifTrue = &fakeLocalIfTrue;
         fakeSelect.ifFalse = &fakeLocalIfFalse;
-        Ref ret = blockify(visitAndAssign(curr->condition, tempCondition));
-        flattenAppend(ret, visitAndAssign(curr->ifTrue, tempIfTrue));
+        fakeSelect.condition = &fakeCondition;
+        Ref ret = blockify(visitAndAssign(curr->ifTrue, tempIfTrue));
         flattenAppend(ret, visitAndAssign(curr->ifFalse, tempIfFalse));
+        flattenAppend(ret, visitAndAssign(curr->condition, tempCondition));
         flattenAppend(ret, visitAndAssign(&fakeSelect, result));
         return ret;
       }
       // normal select
-      Ref condition = visit(curr->condition, EXPRESSION_RESULT);
       Ref ifTrue = visit(curr->ifTrue, EXPRESSION_RESULT);
       Ref ifFalse = visit(curr->ifFalse, EXPRESSION_RESULT);
-      ScopedTemp tempCondition(i32, parent),
-                 tempIfTrue(curr->type, parent),
-                 tempIfFalse(curr->type, parent);
+      Ref condition = visit(curr->condition, EXPRESSION_RESULT);
+      ScopedTemp tempIfTrue(curr->type, parent),
+          tempIfFalse(curr->type, parent),
+          tempCondition(i32, parent);
       return
         ValueBuilder::makeSeq(
           ValueBuilder::makeAssign(tempCondition.getAstName(), condition),
-- 
cgit v1.2.3