Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement br_table; drop tableswitch #249

Merged
merged 4 commits into from
Mar 7, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
17 changes: 5 additions & 12 deletions ml-proto/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ cvtop: trunc_s | trunc_u | extend_s | extend_u | ...
expr:
( nop )
( block <name>? <expr>+ )
( loop <name1>? <name2>? <expr>* ) ;; = (block <name1>? (loop <name2>? (block <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)))
( loop <name1>? <name2>? <expr>* ) ;; = (block <name1>? (loop <name2>? (block <expr>*)))
( if <expr> <expr> ) ;; = (if_else <expr> <expr> (nop))
( br <var> <expr>? )
( return <expr>? ) ;; = (br <current_depth> <expr>?)
( tableswitch <name>? <expr> ( table <target>* ) <target> <case>* )
( br_if <var> <expr>? <expr> )
( br_table <var>* <expr>? <expr> )
( return <expr>? ) ;; = (br <current_depth> <expr>?)
( call <var> <expr>* )
( call_import <var> <expr>* )
( call_indirect <var> <expr> <expr>* )
Expand All @@ -116,13 +116,6 @@ expr:
( memory_size )
( grow_memory <expr> )

target:
( case <var> )
( br <var> ) ;; = (case <name>) with (case <name> (br <var>))

case:
( case <name>? <expr>* ) ;; = (case <var>? (block <expr>*))

func: ( func <name>? <type>? <param>* <result>? <local>* <expr>* )
type: ( type <var> )
param: ( param <type>* ) | ( param <name> <type> )
Expand Down
3 changes: 1 addition & 2 deletions ml-proto/host/lexer.mll
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,10 @@ rule token = parse
| "loop" { LOOP }
| "br" { BR }
| "br_if" { BR_IF }
| "br_table" { BR_TABLE }
| "return" { RETURN }
| "if" { IF }
| "if_else" { IF_ELSE }
| "tableswitch" { TABLESWITCH }
| "case" { CASE }
| "call" { CALL }
| "call_import" { CALL_IMPORT }
| "call_indirect" { CALL_INDIRECT }
Expand Down
38 changes: 7 additions & 31 deletions ml-proto/host/parser.mly
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,17 @@ type types = {mutable tmap : int VarMap.t; mutable tlist : Types.func_type list}
let empty_types () = {tmap = VarMap.empty; tlist = []}

type context =
{types : types; funcs : space; imports : space; locals : space;
labels : int VarMap.t; cases : space}
{types : types; funcs : space; imports : space;
locals : space; labels : int VarMap.t}

let empty_context () =
{types = empty_types (); funcs = empty (); imports = empty ();
locals = empty (); labels = VarMap.empty; cases = empty ()}
locals = empty (); labels = VarMap.empty}

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

let enter_switch c =
{c with cases = empty ()}

let type_ c x =
try VarMap.find x.it c.types.tmap
with Not_found -> error x.at ("unknown type " ^ x.it)
Expand All @@ -76,7 +73,6 @@ let lookup category space x =
let func c x = lookup "function" c.funcs x
let import c x = lookup "import" c.imports x
let local c x = lookup "local" c.locals x
let case c x = lookup "case" c.cases x
let label c x =
try VarMap.find x.it c.labels
with Not_found -> error x.at ("unknown label " ^ x.it)
Expand All @@ -96,7 +92,6 @@ let bind category space x =
let bind_func c x = bind "function" c.funcs x
let bind_import c x = bind "import" c.imports x
let bind_local c x = bind "local" c.locals x
let bind_case c x = bind "case" c.cases x
let bind_label c x =
{c with labels = VarMap.add x.it 0 (VarMap.map ((+) 1) c.labels)}

Expand All @@ -108,7 +103,6 @@ let anon space n = space.count <- space.count + n
let anon_func c = anon c.funcs 1
let anon_import c = anon c.imports 1
let anon_locals c ts = anon c.locals (List.length ts)
let anon_case c = anon c.cases 1
let anon_label c = {c with labels = VarMap.map ((+) 1) c.labels}

let empty_type = {ins = []; out = None}
Expand All @@ -131,7 +125,7 @@ let implicit_decl c t at =
%}

%token INT FLOAT TEXT VAR VALUE_TYPE LPAR RPAR
%token NOP BLOCK IF IF_ELSE LOOP BR BR_IF TABLESWITCH CASE
%token NOP BLOCK IF IF_ELSE LOOP BR BR_IF BR_TABLE
%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 @@ -235,15 +229,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) }
| BR_TABLE var_list expr { fun c -> Br_table ($2 c label, None, $3 c) }
| BR_TABLE var_list expr expr
{ fun c -> Br_table ($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) }
| TABLESWITCH labeling expr LPAR TABLE target_list RPAR target case_list
{ fun c -> let c' = $2 c in let e = $3 c' in
let c'' = enter_switch c' in let es = $9 c'' in
Tableswitch (e, $6 c'', $8 c'', es) }
| CALL var expr_list { fun c -> Call ($2 c func, $3 c) }
| CALL_IMPORT var expr_list { fun c -> Call_import ($2 c import, $3 c) }
| CALL_INDIRECT var expr expr_list
Expand Down Expand Up @@ -271,23 +264,6 @@ expr_list :
| expr expr_list { fun c -> $1 c :: $2 c }
;

target :
| LPAR CASE var RPAR { let at = at () in fun c -> Case ($3 c case) @@ at }
| LPAR BR var RPAR { let at = at () in fun c -> Case_br ($3 c label) @@ at }
;
target_list :
| /* empty */ { fun c -> [] }
| target target_list { fun c -> $1 c :: $2 c }
;
case :
| LPAR CASE expr_list RPAR { fun c -> anon_case c; $3 c }
| LPAR CASE bind_var expr_list RPAR { fun c -> bind_case c $3; $4 c }
;
case_list :
| /* empty */ { fun c -> [] }
| case case_list { fun c -> let e = $1 c in let es = $2 c in e :: es }
;


/* Functions */

Expand Down
5 changes: 1 addition & 4 deletions ml-proto/spec/ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

type var = int Source.phrase

type target = target' Source.phrase
and target' = Case of var | Case_br of var

type expr = expr' Source.phrase
and expr' =
(* Constants *)
Expand All @@ -20,10 +17,10 @@ and expr' =
| Loop of expr list
| Br of var * expr option
| Br_if of var * expr option * expr
| Br_table of var list * expr option * expr
| Return of var * expr option
| If of expr * expr
| If_else of expr * expr * expr
| Tableswitch of expr * target list * target * expr list list
| Call of var * expr list
| Call_import of var * expr list
| Call_indirect of var * expr * expr list
Expand Down
16 changes: 8 additions & 8 deletions ml-proto/spec/check.ml
Original file line number Diff line number Diff line change
Expand Up @@ -133,22 +133,22 @@ let rec check_expr c et e =
| Break (x, eo) ->
check_expr_opt c (label c x) eo e.at

| Br_if (x, eo, e) ->
| Break_if (x, eo, e1) ->
check_expr_opt c (label c x) eo e.at;
check_expr c (Some Int32Type) e;
check_expr c (Some Int32Type) e1;
check_type None et e.at

| Break_table (xs, eo, e1) ->
if xs = [] then check_expr_opt c None eo e.at
else List.iter (fun x -> check_expr_opt c (label c x) eo e.at) xs;
check_expr c (Some Int32Type) e1;
check_type None et e.at

| If (e1, e2, e3) ->
check_expr c (Some Int32Type) e1;
check_expr c et e2;
check_expr c et e3

| Switch (e1, xs, x, es) ->
List.iter (fun x -> require (x.it < List.length es) x.at "invalid target")
(x :: xs);
check_expr c (Some Int32Type) e1;
ignore (List.fold_right (fun e et -> check_expr c et e; None) es et)

| Call (x, es) ->
let {ins; out} = func c x in
check_exprs c ins es e.at;
Expand Down
34 changes: 11 additions & 23 deletions ml-proto/spec/desugar.ml
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@ open Kernel
(* Labels *)

let rec label e = shift 0 e

and shift n e = shift' n e.it @@ e.at
and shift' n = function
| Nop -> Nop
| Unreachable -> Unreachable
| Block es -> Block (List.map (shift (n + 1)) es)
| Loop e -> Loop (shift (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)
| 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)
| Break (x, eo) -> Break (shift_var n x, Lib.Option.map (shift n) eo)
| Break_if (x, eo, e) ->
Break_if (shift_var n x, Lib.Option.map (shift n) eo, shift n e)
| Break_table (xs, eo, e) ->
Break_table
(List.map (shift_var n) xs, Lib.Option.map (shift n) eo, shift n e)
| If (e1, e2, e3) -> If (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)
| CallIndirect (x, e, es) ->
Expand All @@ -41,6 +41,8 @@ and shift' n = function
| Convert (cvtop, e) -> Convert (cvtop, shift n e)
| Host (hostop, es) -> Host (hostop, List.map (shift n) es)

and shift_var n x = if x.it < n then x else (x.it + 1) @@ x.at


(* Expressions *)

Expand All @@ -56,29 +58,15 @@ and expr' at = function
| Ast.Block es -> Block (List.map expr es)
| Ast.Loop es -> Block [Loop (seq 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.Br_if (x, eo, e) -> Break_if (x, Lib.Option.map expr eo, expr e)
| Ast.Br_table (xs, eo, e) -> Break_table (xs, 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.Call (x, es) -> Call (x, List.map expr es)
| Ast.Call_import (x, es) -> CallImport (x, List.map expr es)
| Ast.Call_indirect (x, e, es) -> CallIndirect (x, expr e, List.map expr es)

| Ast.Tableswitch (e, ts, t, es) ->
let target t (xs, es') =
match t.it with
| Ast.Case x -> x :: xs, es'
| Ast.Case_br x ->
(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 n = List.length es' in
let sh x = (if x.it >= n then x.it + n else x.it) @@ x.at in
Block [Switch
(expr e, List.map sh (List.tl xs), sh (List.hd xs), List.rev es' @ es'')
@@ at]

| Ast.Get_local x -> GetLocal x
| Ast.Set_local (x, e) -> SetLocal (x, expr e)

Expand Down
18 changes: 8 additions & 10 deletions ml-proto/spec/eval.ml
Original file line number Diff line number Diff line change
Expand Up @@ -154,24 +154,22 @@ let rec eval_expr (c : config) (e : expr) =
| Break (x, eo) ->
raise (label c x (eval_expr_opt c eo))

| Br_if (x, eo, e) ->
| Break_if (x, eo, e) ->
let v = eval_expr_opt c eo in
let i = int32 (eval_expr c e) e.at in
if i <> 0l then raise (label c x v) else None

| Break_table (xs, eo, e) ->
let v = eval_expr_opt c eo in
let i = int32 (eval_expr c e) e.at in
if I32.lt_u i (Int32.of_int (List.length xs))
then raise (label c (List.nth xs (Int32.to_int i)) v)
else None

| If (e1, e2, e3) ->
let i = int32 (eval_expr c e1) e1.at in
eval_expr c (if i <> 0l then e2 else e3)

| Switch (e1, xs, x, es) ->
let i = int32 (eval_expr c e1) e1.at in
let x' =
if I32.ge_u i (Int32.of_int (List.length xs)) then x
else List.nth xs (Int32.to_int i)
in
if x'.it >= List.length es then Crash.error e.at "invalid switch target";
List.fold_left (fun vo e -> eval_expr c e) None (Lib.List.drop x'.it es)

| Call (x, es) ->
let vs = List.map (fun vo -> some (eval_expr c vo) vo.at) es in
eval_func c.instance (func c x) vs
Expand Down
4 changes: 2 additions & 2 deletions ml-proto/spec/kernel.ml
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ and expr' =
| Block of expr list (* execute in sequence *)
| Loop of expr (* loop header *)
| Break of var * expr option (* break to n-th surrounding label *)
| Br_if of var * expr option * expr (* conditional break *)
| Break_if of var * expr option * expr (* conditional break *)
| Break_table of var list * expr option * expr (* indexed break *)
Copy link
Member

Choose a reason for hiding this comment

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

Naming these Break_if and Break_table, when the ast.ml names are Br_if and Br_table, makes it less obvious that kernel.ml is intended to be a subset of ast.ml.

Copy link
Member Author

Choose a reason for hiding this comment

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

Remember that this is abstract syntax, not concrete, so it does not need to reflect naming 1:1.

But you are right that the names weren't particularly consistent -- I switched to proper camel casing, like the other constructors use. :)

Copy link
Member

Choose a reason for hiding this comment

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

This is more of a pre-existing nit: but they're not breaks; breaks only branch forward and these branch forwards and backwards. How about expanding to "br" to "Branch"? That way noone will be superficially confused by lack of "Continue" :)

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, but there are arguments both ways on that; not sure we should revisit the br naming debate (WebAssembly/design#445) here.

Copy link
Member

Choose a reason for hiding this comment

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

That was a text-format discussion and I think, in that proposal, the user would see both "break" and "continue" (therefore going the correct direction) so that's a separate topic altogether; here we have a single node that is jumping both forward and backward.

Copy link
Member

Choose a reason for hiding this comment

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

My point is many of the arguments there are also applicable here, both ways, and we couldn't resolve things there.

| If of expr * expr * expr (* conditional *)
| Switch of expr * var list * var * expr list (* table switch *)
| Call of var * expr list (* call function *)
| CallImport of var * expr list (* call imported function *)
| CallIndirect of var * expr * expr list (* call function through table *)
Expand Down
42 changes: 29 additions & 13 deletions ml-proto/test/labels.wast
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,41 @@
(func $switch (param i32) (result i32)
(block $ret
(i32.mul (i32.const 10)
(tableswitch $exit (get_local 0)
(table (case $0) (case $1) (case $2) (case $3)) (case $default)
(case $1 (i32.const 1))
(case $2 (br $exit (i32.const 2)))
(case $3 (br $ret (i32.const 3)))
(case $default (i32.const 4))
(case $0 (i32.const 5))
(block $exit
(block $0
(block $default
(block $3
(block $2
(block $1
(br_table $0 $1 $2 $3 (get_local 0))
(br $default)
) ;; 1
(i32.const 1)
) ;; 2
(br $exit (i32.const 2))
) ;; 3
(br $ret (i32.const 3))
) ;; default
(i32.const 4)
) ;; 0
(i32.const 5)
)
)
)
)

(func $return (param i32) (result i32)
(tableswitch (get_local 0)
(table (case $0) (case $1)) (case $default)
(case $0 (return (i32.const 0)))
(case $1 (i32.const 1))
(case $default (i32.const 2))
)
(block $default
(block $1
(block $0
(br_table $0 $1 (get_local 0))
(br $default)
) ;; 0
(return (i32.const 0))
) ;; 1
(i32.const 1)
) ;; default
(i32.const 2)
)

(func $br_if0 (result i32)
Expand Down
Loading