Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix return representation in Ast #256

Merged
merged 6 commits into from
Mar 2, 2016
Merged

Fix return representation in Ast #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c717445
Fix `return` representation in Ast
rossberg Feb 24, 2016
0568800
Adjust `if` to match post-order format.
rossberg Feb 24, 2016
acb05eb
Typo
rossberg Feb 25, 2016
f4129aa
Merge pull request #257 from WebAssembly/if
rossberg Mar 2, 2016
436e8ca
Merge branch 'master' into return
rossberg Mar 2, 2016
0f5f268
Merge branch 'return' of https://github.com/WebAssembly/spec into return
rossberg Mar 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ml-proto/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ expr:
( nop )
( block <name>? <expr>* )
( select <expr> <expr> <expr> )
( if_else <expr> <expr> <expr> )
( if <expr> <expr> ) ;; = (if_else <expr> <expr> (nop))
( br_if <expr> <var> <expr>?) ;; = (if_else <expr> (br <var> <expr>?) (block <expr>? (nop)))
( if <expr> ( then <name>? <expr>* ) ( else <name>? <expr>* )? )
( if <expr1> <expr2> <expr3>? ) ;; = (if <expr1> (then <expr2>) (else <expr3>?))
( br_if <expr> <var> <expr>? )
( loop <name1>? <name2>? <expr>* ) ;; = (block <name1>? (loop <name2>? (block <expr>*)))
( br <var> <expr>? )
( return <expr>? ) ;; = (br <current_depth> <expr>?)
Expand Down
3 changes: 2 additions & 1 deletion ml-proto/host/lexer.mll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ rule token = parse
| "br_if" { BR_IF }
| "return" { RETURN }
| "if" { IF }
| "if_else" { IF_ELSE }
| "then" { THEN }
| "else" { ELSE }
| "select" { SELECT }
| "tableswitch" { TABLESWITCH }
| "case" { CASE }
Expand Down
20 changes: 12 additions & 8 deletions ml-proto/host/parser.mly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ let empty_context () =

let enter_func c =
assert (VarMap.is_empty c.labels);
{c with labels = VarMap.add "return" 0 c.labels; locals = empty ()}
{c with labels = VarMap.empty; locals = empty ()}

let enter_switch c =
{c with cases = empty ()}
Expand Down Expand Up @@ -131,7 +131,7 @@ let implicit_decl c t at =
%}

%token INT FLOAT TEXT VAR VALUE_TYPE LPAR RPAR
%token NOP BLOCK IF IF_ELSE SELECT LOOP BR BR_IF TABLESWITCH CASE
%token NOP BLOCK IF THEN ELSE SELECT LOOP BR BR_IF TABLESWITCH CASE
%token CALL CALL_IMPORT CALL_INDIRECT RETURN
%token GET_LOCAL SET_LOCAL LOAD STORE OFFSET ALIGN
%token CONST UNARY BINARY COMPARE CONVERT
Expand Down Expand Up @@ -233,11 +233,14 @@ expr1 :
| BR var expr_opt { fun c -> Br ($2 c label, $3 c) }
| BR_IF var expr { fun c -> Br_if ($2 c label, None, $3 c) }
| BR_IF var expr expr { fun c -> Br_if ($2 c label, Some ($3 c), $4 c) }
| RETURN expr_opt
{ let at1 = ati 1 in
fun c -> Return (label c ("return" @@ at1) @@ at1, $2 c) }
| IF expr expr { fun c -> If ($2 c, $3 c) }
| IF_ELSE expr expr expr { fun c -> If_else ($2 c, $3 c, $4 c) }
| RETURN expr_opt { fun c -> Return ($2 c) }
| IF expr expr { fun c -> let c' = anon_label c in If ($2 c, [$3 c'], []) }
| IF expr expr expr
{ fun c -> let c' = anon_label c in If ($2 c, [$3 c'], [$4 c']) }
| IF expr LPAR THEN labeling expr_list RPAR
{ fun c -> let c' = $5 c in If ($2 c, $6 c', []) }
| IF expr LPAR THEN labeling expr_list RPAR LPAR ELSE labeling expr_list RPAR
{ fun c -> let c1 = $5 c in let c2 = $10 c in If ($2 c, $6 c1, $11 c2) }
| SELECT expr expr expr { fun c -> Select ($2 c, $3 c, $4 c) }
| TABLESWITCH labeling expr LPAR TABLE target_list RPAR target case_list
{ fun c -> let c' = $2 c in let e = $3 c' in
Expand Down Expand Up @@ -291,7 +294,8 @@ case_list :
func_fields :
| expr_list
{ empty_type,
fun c -> {ftype = -1 @@ at(); locals = []; body = $1 c} }
fun c -> let c' = anon_label c in
{ftype = -1 @@ at(); locals = []; body = $1 c'} }
| LPAR PARAM value_type_list RPAR func_fields
{ {(fst $5) with ins = $3 @ (fst $5).ins},
fun c -> anon_locals c $3; (snd $5) c }
Expand Down
5 changes: 2 additions & 3 deletions ml-proto/spec/ast.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ and expr' =
| Loop of expr list
| Br of var * expr option
| Br_if of var * expr option * expr
| Return of var * expr option
| If of expr * expr
| If_else of expr * expr * expr
| Return of expr option
| If of expr * expr list * expr list
| Select of expr * expr * expr
| Tableswitch of expr * target list * target * expr list list
| Call of var * expr list
Expand Down
79 changes: 40 additions & 39 deletions ml-proto/spec/desugar.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,38 +7,42 @@ open Kernel

(* Labels *)

let rec label e = shift 0 e
and shift n e = shift' n e.it @@ e.at
and shift' n = function
let rec relabel f n e = relabel' f n e.it @@ e.at
and relabel' f n = function
| Nop -> Nop
| Unreachable -> Unreachable
| Block (es, e) -> Block (List.map (shift (n + 1)) es, shift (n + 1) e)
| Loop e -> Loop (shift (n + 1) e)
| Block (es, e) ->
Block (List.map (relabel f (n + 1)) es, relabel f (n + 1) e)
| Loop e -> Loop (relabel f (n + 1) e)
| Break (x, eo) ->
let x' = if x.it < n then x else (x.it + 1) @@ x.at in
Break (x', Lib.Option.map (shift n) eo)
Break (f n x.it @@ x.at, Lib.Option.map (relabel f n) eo)
| Br_if (x, eo, e) ->
let x' = if x.it < n then x else (x.it + 1) @@ x.at in
Br_if (x', Lib.Option.map (shift n) eo, shift n e)
| If (e1, e2, e3) -> If (shift n e1, shift n e2, shift n e3)
| Select (e1, e2, e3) -> Select (shift n e1, shift n e2, shift n e3)
| Switch (e, xs, x, es) -> Switch (shift n e, xs, x, List.map (shift n) es)
| Call (x, es) -> Call (x, List.map (shift n) es)
| CallImport (x, es) -> CallImport (x, List.map (shift n) es)
Br_if (f n x.it @@ x.at, Lib.Option.map (relabel f n) eo, relabel f n e)
| If (e1, e2, e3) -> If (relabel f n e1, relabel f n e2, relabel f n e3)
| Select (e1, e2, e3) ->
Select (relabel f n e1, relabel f n e2, relabel f n e3)
| Switch (e, xs, x, es) ->
Switch (relabel f n e, xs, x, List.map (relabel f n) es)
| Call (x, es) -> Call (x, List.map (relabel f n) es)
| CallImport (x, es) -> CallImport (x, List.map (relabel f n) es)
| CallIndirect (x, e, es) ->
CallIndirect (x, shift n e, List.map (shift n) es)
CallIndirect (x, relabel f n e, List.map (relabel f n) es)
| GetLocal x -> GetLocal x
| SetLocal (x, e) -> SetLocal (x, shift n e)
| Load (memop, e) -> Load (memop, shift n e)
| Store (memop, e1, e2) -> Store (memop, shift n e1, shift n e2)
| LoadExtend (extop, e) -> LoadExtend (extop, shift n e)
| StoreWrap (wrapop, e1, e2) -> StoreWrap (wrapop, shift n e1, shift n e2)
| SetLocal (x, e) -> SetLocal (x, relabel f n e)
| Load (memop, e) -> Load (memop, relabel f n e)
| Store (memop, e1, e2) -> Store (memop, relabel f n e1, relabel f n e2)
| LoadExtend (extop, e) -> LoadExtend (extop, relabel f n e)
| StoreWrap (wrapop, e1, e2) ->
StoreWrap (wrapop, relabel f n e1, relabel f n e2)
| Const c -> Const c
| Unary (unop, e) -> Unary (unop, shift n e)
| Binary (binop, e1, e2) -> Binary (binop, shift n e1, shift n e2)
| Compare (relop, e1, e2) -> Compare (relop, shift n e1, shift n e2)
| Convert (cvtop, e) -> Convert (cvtop, shift n e)
| Host (hostop, es) -> Host (hostop, List.map (shift n) es)
| Unary (unop, e) -> Unary (unop, relabel f n e)
| Binary (binop, e1, e2) -> Binary (binop, relabel f n e1, relabel f n e2)
| Compare (relop, e1, e2) -> Compare (relop, relabel f n e1, relabel f n e2)
| Convert (cvtop, e) -> Convert (cvtop, relabel f n e)
| Host (hostop, es) -> Host (hostop, List.map (relabel f n) es)

let label e = relabel (fun n i -> if i < n then i else i + 1) 0 e
let return e = relabel (fun n i -> if i = -1 then n else i) (-1) e


(* Expressions *)
Expand All @@ -55,12 +59,11 @@ and expr' at = function
| Ast.Block [] -> Nop
| Ast.Block es ->
let es', e = Lib.List.split_last es in Block (List.map expr es', expr e)
| Ast.Loop es -> Block ([], Loop (seq es) @@ at)
| Ast.Loop es -> Block ([], Loop (block es) @@ at)
| Ast.Br (x, eo) -> Break (x, Lib.Option.map expr eo)
| Ast.Br_if (x, eo, e) -> Br_if (x, Lib.Option.map expr eo, expr e)
| Ast.Return (x, eo) -> Break (x, Lib.Option.map expr eo)
| Ast.If (e1, e2) -> If (expr e1, expr e2, Nop @@ Source.after e2.at)
| Ast.If_else (e1, e2, e3) -> If (expr e1, expr e2, expr e3)
| Ast.Return eo -> Break (-1 @@ Source.no_region, Lib.Option.map expr eo)
| Ast.If (e, es1, es2) -> If (expr e, seq es1, seq es2)
| Ast.Select (e1, e2, e3) -> Select (expr e1, expr e2, expr e3)

| Ast.Call (x, es) -> Call (x, List.map expr es)
Expand All @@ -75,7 +78,7 @@ and expr' at = function
(List.length es' @@ t.at) :: xs, (Break (x, None) @@ t.at) :: es'
in
let xs, es' = List.fold_right target (t :: ts) ([], []) in
let es'' = List.map seq es in
let es'' = List.map block es in
let n = List.length es' in
let sh x = (if x.it >= n then x.it + n else x.it) @@ x.at in
Block ([], Switch
Expand Down Expand Up @@ -284,25 +287,23 @@ and expr' at = function

and seq = function
| [] -> Nop @@ Source.no_region
| [e] -> expr e
| es ->
let es', e = Lib.List.split_last es in
Block (List.map expr es', expr e) @@@ List.map Source.at es

and block = function
| [] -> Nop @@ Source.no_region
| es ->
let es', e = Lib.List.split_last es in
Block (List.map label (List.map expr es'), label (expr e))
@@@ List.map Source.at es

and opt = function
| None -> Nop @@ Source.no_region
| Some e -> Block ([label (expr e)], Nop @@ e.at) @@ e.at


(* Functions and Modules *)

let rec func f = func' f.it @@ f.at
and func' = function
| {Ast.body = []; ftype; locals} ->
{body = Nop @@ no_region; ftype; locals}
| {Ast.body = es; ftype; locals} ->
{body = Block ([], seq es) @@@ List.map at es; ftype; locals}
| {Ast.body = es; ftype; locals} -> {body = return (seq es); ftype; locals}

let rec module_ m = module' m.it @@ m.at
and module' = function
Expand Down
10 changes: 5 additions & 5 deletions ml-proto/test/fac.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
(module
;; Recursive factorial
(func (param i64) (result i64)
(if_else (i64.eq (get_local 0) (i64.const 0))
(if (i64.eq (get_local 0) (i64.const 0))
(i64.const 1)
(i64.mul (get_local 0) (call 0 (i64.sub (get_local 0) (i64.const 1))))
)
)

;; Recursive factorial named
(func $fac-rec (param $n i64) (result i64)
(if_else (i64.eq (get_local $n) (i64.const 0))
(if (i64.eq (get_local $n) (i64.const 0))
(i64.const 1)
(i64.mul
(get_local $n)
Expand All @@ -24,9 +24,9 @@
(set_local 1 (get_local 0))
(set_local 2 (i64.const 1))
(loop
(if_else
(if
(i64.eq (get_local 1) (i64.const 0))
(br 1)
(br 2)
(block
(set_local 2 (i64.mul (get_local 1) (get_local 2)))
(set_local 1 (i64.sub (get_local 1) (i64.const 1)))
Expand All @@ -44,7 +44,7 @@
(set_local $i (get_local $n))
(set_local $res (i64.const 1))
(loop $done $loop
(if_else
(if
(i64.eq (get_local $i) (i64.const 0))
(br $done)
(block
Expand Down
4 changes: 2 additions & 2 deletions ml-proto/test/forward.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
(export "odd" $odd)

(func $even (param $n i32) (result i32)
(if_else (i32.eq (get_local $n) (i32.const 0))
(if (i32.eq (get_local $n) (i32.const 0))
(i32.const 1)
(call $odd (i32.sub (get_local $n) (i32.const 1)))
)
)

(func $odd (param $n i32) (result i32)
(if_else (i32.eq (get_local $n) (i32.const 0))
(if (i32.eq (get_local $n) (i32.const 0))
(i32.const 0)
(call $even (i32.sub (get_local $n) (i32.const 1)))
)
Expand Down
49 changes: 46 additions & 3 deletions ml-proto/test/labels.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,43 @@
)
)

(func $if (result i32)
(local $i i32)
(set_local $i (i32.const 0))
(block
(if
(i32.const 1)
(then $l (br $l) (set_local $i (i32.const 666)))
)
(set_local $i (i32.add (get_local $i) (i32.const 1)))
(if
(i32.const 1)
(then $l (br $l) (set_local $i (i32.const 666)))
(else (set_local $i (i32.const 888)))
)
(set_local $i (i32.add (get_local $i) (i32.const 1)))
(if
(i32.const 1)
(then $l (br $l) (set_local $i (i32.const 666)))
(else $l (set_local $i (i32.const 888)))
)
(set_local $i (i32.add (get_local $i) (i32.const 1)))
(if
(i32.const 0)
(then (set_local $i (i32.const 888)))
(else $l (br $l) (set_local $i (i32.const 666)))
)
(set_local $i (i32.add (get_local $i) (i32.const 1)))
(if
(i32.const 0)
(then $l (set_local $i (i32.const 888)))
(else $l (br $l) (set_local $i (i32.const 666)))
)
(set_local $i (i32.add (get_local $i) (i32.const 1)))
)
(get_local $i)
)

(func $switch (param i32) (result i32)
(block $ret
(i32.mul (i32.const 10)
Expand Down Expand Up @@ -100,9 +137,13 @@
(br_if $inner (i32.const 1))
(set_local $i (i32.or (get_local $i) (i32.const 0x2)))
)
(br_if $outer (set_local $i (i32.or (get_local $i) (i32.const 0x4))) (i32.const 0))
(br_if $outer
(set_local $i (i32.or (get_local $i) (i32.const 0x4))) (i32.const 0)
)
(set_local $i (i32.or (get_local $i) (i32.const 0x8)))
(br_if $outer (set_local $i (i32.or (get_local $i) (i32.const 0x10))) (i32.const 1))
(br_if $outer
(set_local $i (i32.or (get_local $i) (i32.const 0x10))) (i32.const 1)
)
(set_local $i (i32.or (get_local $i) (i32.const 0x20)))
)
)
Expand Down Expand Up @@ -152,6 +193,7 @@
(export "loop3" $loop3)
(export "loop4" $loop4)
(export "loop5" $loop5)
(export "if" $if)
(export "switch" $switch)
(export "return" $return)
(export "br_if0" $br_if0)
Expand All @@ -169,6 +211,7 @@
(assert_return (invoke "loop3") (i32.const 1))
(assert_return (invoke "loop4" (i32.const 8)) (i32.const 16))
(assert_return (invoke "loop5") (i32.const 2))
(assert_return (invoke "if") (i32.const 5))
(assert_return (invoke "switch" (i32.const 0)) (i32.const 50))
(assert_return (invoke "switch" (i32.const 1)) (i32.const 20))
(assert_return (invoke "switch" (i32.const 2)) (i32.const 20))
Expand Down Expand Up @@ -200,7 +243,7 @@
"arity mismatch")
(assert_invalid (module (func (result i32)
(block $l0
(if_else (i32.const 1)
(if (i32.const 1)
(br $l0 (block $l1 (br $l1 (i32.const 1))))
(block (block $l1 (br $l1 (i32.const 1))) (nop))
)
Expand Down
4 changes: 2 additions & 2 deletions ml-proto/test/memory.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
(loop
(if
(i32.eq (get_local 0) (i32.const 0))
(br 1)
(br 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull is for return - why did these brs change?

And where does 2 go? the only blocks or loops above this br are the one loop, which would take indexes 0 and 1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if has an implicit label now too. Change was here: 0568800. Not sure why it didn't break then.

edit: oh, it got added in to this merge accidentally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! now i see

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for this, I messed up the branching, and accidentally merged the if changes into the return branch instead of master. This line is part of the if changes (if now has an implicit block).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it the if that has an implicit block, or the then and else branches that do? because I see they can have a label name on them now? must it be the same for both of them if there is?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branches have implicit blocks, so the labels go there (also, that makes it clear that they are not in scope in the condition). They can be named differently, see the examples in test/labels.wast.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, thanks!

)
(set_local 2 (i32.mul (get_local 0) (i32.const 4)))
(i32.store (get_local 2) (get_local 0))
Expand All @@ -117,7 +117,7 @@
(loop
(if
(i32.eq (get_local 0) (i32.const 0))
(br 1)
(br 2)
)
(set_local 2 (f64.convert_s/i32 (get_local 0)))
(f64.store align=1 (get_local 0) (get_local 2))
Expand Down
2 changes: 1 addition & 1 deletion ml-proto/test/unreachable.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
(unreachable))

(func $if (param i32) (result f32)
(if_else (get_local 0) (unreachable) (f32.const 0)))
(if (get_local 0) (unreachable) (f32.const 0)))

(func $block
(block (i32.const 1) (unreachable) (i32.const 2)))
Expand Down