From 82b0a889fd2534af81e21c277657adf58699d73c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Fri, 20 Mar 2015 12:20:52 +0100
Subject: Remove support for put_map_exact without a source map

Using the exact operator (':=') is only allowed when an existing map
is being updated. Thus the following causes a compilation error:

  #{k:=v}

Therefore there is no need to support the put_map_exact instruction
without a source map.
---
 erts/emulator/beam/ops.tab | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index d3649080dc..ae3b7d08b8 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1478,7 +1478,7 @@ put_map_assoc F Src=s Dst Live Size Rest=* => \
 	       update_map_assoc F Src Dst Live Size Rest
 put_map_assoc F Src Dst Live Size Rest=* => \
 	       move Src x | update_map_assoc F x Dst Live Size Rest
-put_map_exact F n Dst Live Size Rest=* => new_map F Dst Live Size Rest
+
 put_map_exact F Src=s Dst Live Size Rest=* => \
 	      update_map_exact F Src Dst Live Size Rest
 put_map_exact F Src Dst Live Size Rest=* => \
-- 
cgit v1.2.3


From 8c5a577ed55a607841989763d78b3950eebd5b5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Fri, 20 Mar 2015 14:45:31 +0100
Subject: Correct transformation of put_map_assoc to new_map

A put_map_assoc instruction with an empty source map should be
converted to a simpler new_map instruction. The transformation
didn't happen because an empty source map is no longer represented
as a NIL term (as it was in the beginning before map literals
were implemented).
---
 erts/emulator/beam/beam_load.c | 17 +++++++++++++++++
 erts/emulator/beam/ops.tab     |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 02689e5b19..f140bb54cc 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -36,6 +36,7 @@
 #include "beam_catches.h"
 #include "erl_binary.h"
 #include "erl_zlib.h"
+#include "erl_map.h"
 
 #ifdef HIPE
 #include "hipe_bif0.h"
@@ -4050,6 +4051,22 @@ tuple_append_put(LoaderState* stp, GenOpArg Arity, GenOpArg Dst,
     return op;
 }
 
+/*
+ * Predicate to test whether the given literal is an empty map.
+ */
+
+static int
+is_empty_map(LoaderState* stp, GenOpArg Lit)
+{
+    Eterm term;
+
+    if (Lit.type != TAG_q) {
+	return 0;
+    }
+    term = stp->literals[Lit.val].term;
+    return is_flatmap(term) && flatmap_get_size(flatmap_val(term)) == 0;
+}
+
 /*
  * Replace a get_map_elements with one key to an instruction with one
  * element
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index ae3b7d08b8..2c4458ae74 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1473,7 +1473,8 @@ apply_last I P
 # Map instructions in R17.
 #
 
-put_map_assoc F n Dst Live Size Rest=* => new_map F Dst Live Size Rest
+put_map_assoc F Map Dst Live Size Rest=* | is_empty_map(Map) => \
+   new_map F Dst Live Size Rest
 put_map_assoc F Src=s Dst Live Size Rest=* => \
 	       update_map_assoc F Src Dst Live Size Rest
 put_map_assoc F Src Dst Live Size Rest=* => \
-- 
cgit v1.2.3


From ee91c4291212035b8e054072407c74afdd66f1d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Fri, 20 Mar 2015 15:07:24 +0100
Subject: Remove the fail label operand of the new_map instruction

The new_map instruction cannot fail, and thus needs no fail label.
---
 erts/emulator/beam/beam_debug.c | 2 +-
 erts/emulator/beam/beam_emu.c   | 8 ++++----
 erts/emulator/beam/ops.tab      | 6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_debug.c b/erts/emulator/beam/beam_debug.c
index 6bb987985d..38e54e9d1a 100644
--- a/erts/emulator/beam/beam_debug.c
+++ b/erts/emulator/beam/beam_debug.c
@@ -661,7 +661,7 @@ print_op(int to, void *to_arg, int op, int size, BeamInstr* addr)
     case op_i_put_tuple_rI:
     case op_i_put_tuple_xI:
     case op_i_put_tuple_yI:
-    case op_new_map_jdII:
+    case op_new_map_dII:
     case op_update_map_assoc_jsdII:
     case op_update_map_exact_jsdII:
     case op_i_has_map_fields_fsI:
diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index fdb84aae42..a264669e50 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -2379,16 +2379,16 @@ void process_main(void)
      Goto(*I);
  }
 
- OpCase(new_map_jdII): {
+ OpCase(new_map_dII): {
      Eterm res;
 
      x(0) = r(0);
      SWAPOUT;
-     res = new_map(c_p, reg, I);
+     res = new_map(c_p, reg, I-1);
      SWAPIN;
      r(0) = x(0);
-     StoreResult(res, Arg(1));
-     Next(4+Arg(3));
+     StoreResult(res, Arg(0));
+     Next(3+Arg(2));
  }
 
  OpCase(i_has_map_fields_fsI): {
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 2c4458ae74..7993dd9e25 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1473,8 +1473,8 @@ apply_last I P
 # Map instructions in R17.
 #
 
-put_map_assoc F Map Dst Live Size Rest=* | is_empty_map(Map) => \
-   new_map F Dst Live Size Rest
+put_map_assoc j Map Dst Live Size Rest=* | is_empty_map(Map) => \
+   new_map Dst Live Size Rest
 put_map_assoc F Src=s Dst Live Size Rest=* => \
 	       update_map_assoc F Src Dst Live Size Rest
 put_map_assoc F Src Dst Live Size Rest=* => \
@@ -1485,7 +1485,7 @@ put_map_exact F Src=s Dst Live Size Rest=* => \
 put_map_exact F Src Dst Live Size Rest=* => \
 	      move Src x | update_map_exact F x Dst Live Size Rest
 
-new_map j d I I
+new_map d I I
 update_map_assoc j s d I I
 update_map_exact j s d I I
 
-- 
cgit v1.2.3


From a1e409e082392fcb8dc8131b77851d72551711a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 23 Mar 2015 09:56:06 +0100
Subject: Run a clone of map_SUITE without optimizations

Create a clone of map_SUITE named map_no_opt_SUITE to ensure that
the loader can cope with unoptimized map instructions.
---
 erts/emulator/test/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/test/Makefile b/erts/emulator/test/Makefile
index dd2e2cb504..4dc4b5027d 100644
--- a/erts/emulator/test/Makefile
+++ b/erts/emulator/test/Makefile
@@ -125,7 +125,8 @@ NO_OPT= bs_bincomp \
 	bs_match_tail \
 	bs_match_misc \
 	bs_utf \
-	guard
+	guard \
+	map
 
 NATIVE= hibernate
 
-- 
cgit v1.2.3


From cd4c3e3bc699e73a7bada55a74333e5a09c7f9e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 23 Mar 2015 10:07:21 +0100
Subject: map_SUITE: Add tests of is_map/1 with literal maps

To be sure that the compiler and BEAM virtual machine correctly
handles literals maps, we must test it.
---
 erts/emulator/test/map_SUITE.erl | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/test/map_SUITE.erl b/erts/emulator/test/map_SUITE.erl
index ad8411cd68..490dd0c1ad 100644
--- a/erts/emulator/test/map_SUITE.erl
+++ b/erts/emulator/test/map_SUITE.erl
@@ -38,6 +38,7 @@
 	t_map_equal/1,
 	t_map_compare/1,
 	t_map_size/1,
+	t_is_map/1,
 
 	%% Specific Map BIFs
 	t_bif_map_get/1,
@@ -114,7 +115,7 @@ all() -> [
 
 	%% erlang
 	t_erlang_hash, t_map_encode_decode,
-	t_map_size,
+	t_map_size, t_is_map,
 
 	%% non specific BIF related
 	t_bif_build_and_check,
@@ -680,6 +681,17 @@ build_and_check_size([],N,M) ->
 map_is_size(M,N) when map_size(M) =:= N -> true;
 map_is_size(_,_) -> false.
 
+t_is_map(Config) when is_list(Config) ->
+    true = is_map(#{}),
+    true = is_map(#{a=>1}),
+    false = is_map({a,b}),
+    false = is_map(x),
+    if is_map(#{}) -> ok end,
+    if is_map(#{b=>1}) -> ok end,
+    if not is_map([1,2,3]) -> ok end,
+    if not is_map(x) -> ok end,
+    ok.
+
 % test map updates without matching
 t_update_literals_large(Config) when is_list(Config) ->
     Map = id(#{ 10=>id(a0),20=>b0,30=>id("c0"),"40"=>"d0",<<"50">>=>id("e0"),{["00"]}=>"10",
-- 
cgit v1.2.3


From 85afb5e3441e78cd0d572dc809d94cfc810d71ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 23 Mar 2015 10:24:56 +0100
Subject: Fully evaluate is_map/1 for literals at load-time

The compiler will only emit is_map/1 instructions with literal
argument if optimization is turned off. Therefore, the only
reason for this commit is cleanliness.
---
 erts/emulator/beam/beam_load.c | 14 ++++++++++++++
 erts/emulator/beam/ops.tab     |  4 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index f140bb54cc..60f4ab5280 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -4051,6 +4051,20 @@ tuple_append_put(LoaderState* stp, GenOpArg Arity, GenOpArg Dst,
     return op;
 }
 
+/*
+ * Predicate to test whether the given literal is a map.
+ */
+
+static int
+literal_is_map(LoaderState* stp, GenOpArg Lit)
+{
+    Eterm term;
+
+    ASSERT(Lit.type == TAG_q);
+    term = stp->literals[Lit.val].term;
+    return is_map(term);
+}
+
 /*
  * Predicate to test whether the given literal is an empty map.
  */
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 7993dd9e25..a5a89b3990 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1489,8 +1489,8 @@ new_map d I I
 update_map_assoc j s d I I
 update_map_exact j s d I I
 
-is_map Fail Literal=q => move Literal x | is_map Fail x
-is_map Fail c => jump Fail
+is_map Fail Lit=q | literal_is_map(Lit) =>
+is_map Fail cq => jump Fail
 
 %macro: is_map IsMap -fail_action
 is_map f r
-- 
cgit v1.2.3


From fea64943212cd96764b595c91b22da68bc72e999 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 23 Mar 2015 11:49:19 +0100
Subject: erts/map_SUITE.erl: Add a test case that tests has_map_fields

The has_map_fields instruction was not tested at all by
erts/map_SUITE.erl
---
 erts/emulator/test/map_SUITE.erl | 44 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/test/map_SUITE.erl b/erts/emulator/test/map_SUITE.erl
index 490dd0c1ad..7b68e91dc5 100644
--- a/erts/emulator/test/map_SUITE.erl
+++ b/erts/emulator/test/map_SUITE.erl
@@ -75,7 +75,10 @@
 	t_pdict/1,
 	t_ets/1,
 	t_dets/1,
-	t_tracing/1
+	t_tracing/1,
+
+	%% instruction-level tests
+	t_has_map_fields/1
     ]).
 
 -include_lib("stdlib/include/ms_transform.hrl").
@@ -132,7 +135,10 @@ all() -> [
         t_erts_internal_hash,
 	t_pdict,
 	t_ets,
-	t_tracing
+	t_tracing,
+
+	%% instruction-level tests
+	t_has_map_fields
     ].
 
 groups() -> [].
@@ -2715,5 +2721,39 @@ trace_collector(Msg,Parent) ->
     Parent ! Msg,
     Parent.
 
+t_has_map_fields(Config) when is_list(Config) ->
+    true = has_map_fields_1(#{one=>1}),
+    true = has_map_fields_1(#{one=>1,two=>2}),
+    false = has_map_fields_1(#{two=>2}),
+    false = has_map_fields_1(#{}),
+
+    true = has_map_fields_2(#{c=>1,b=>2,a=>3}),
+    true = has_map_fields_2(#{c=>1,b=>2,a=>3,x=>42}),
+    false = has_map_fields_2(#{b=>2,c=>1}),
+    false = has_map_fields_2(#{x=>y}),
+    false = has_map_fields_2(#{}),
+
+    true = has_map_fields_3(#{c=>1,b=>2,a=>3}),
+    true = has_map_fields_3(#{c=>1,b=>2,a=>3,[]=>42}),
+    true = has_map_fields_3(#{b=>2,a=>3,[]=>42,42.0=>43}),
+    true = has_map_fields_3(#{a=>3,[]=>42,42.0=>43}),
+    true = has_map_fields_3(#{[]=>42,42.0=>43}),
+    false = has_map_fields_3(#{b=>2,c=>1}),
+    false = has_map_fields_3(#{[]=>y}),
+    false = has_map_fields_3(#{42.0=>x,a=>99}),
+    false = has_map_fields_3(#{}),
+
+    ok.
+
+has_map_fields_1(#{one:=_}) -> true;
+has_map_fields_1(#{}) -> false.
+
+has_map_fields_2(#{a:=_,b:=_,c:=_}) -> true;
+has_map_fields_2(#{}) -> false.
+
+has_map_fields_3(#{a:=_,b:=_}) -> true;
+has_map_fields_3(#{[]:=_,42.0:=_}) -> true;
+has_map_fields_3(#{}) -> false.
+
 %% Use this function to avoid compile-time evaluation of an expression.
 id(I) -> I.
-- 
cgit v1.2.3


From c92cf260bb888c004fb02651670d19989dbc2b74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 26 Mar 2015 13:16:45 +0100
Subject: De-optimize the has_map_fields instructions

The has_map_fields instruction is infrequently used. Thus there
is no need to have the fastest possible implementation; it is
better to have an implementation that reduces the code size in
the already big process_main() function.

We can transform has_map_fields to a get_map_elements instruction,
targeting the same unused x[0] register for all keys. That
instruction will only be marginally slower than existing
implementation.
---
 erts/emulator/beam/beam_debug.c |  1 -
 erts/emulator/beam/beam_emu.c   | 96 -----------------------------------------
 erts/emulator/beam/beam_load.c  | 20 ++++++---
 erts/emulator/beam/ops.tab      | 29 ++-----------
 4 files changed, 19 insertions(+), 127 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_debug.c b/erts/emulator/beam/beam_debug.c
index 38e54e9d1a..0367ca8aba 100644
--- a/erts/emulator/beam/beam_debug.c
+++ b/erts/emulator/beam/beam_debug.c
@@ -664,7 +664,6 @@ print_op(int to, void *to_arg, int op, int size, BeamInstr* addr)
     case op_new_map_dII:
     case op_update_map_assoc_jsdII:
     case op_update_map_exact_jsdII:
-    case op_i_has_map_fields_fsI:
     case op_i_get_map_elements_fsI:
 	{
 	    int n = unpacked[-1];
diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index a264669e50..f25b9f594d 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -701,8 +701,6 @@ void** beam_ops;
 
 #define IsMap(Src, Fail) if (!is_map(Src)) { Fail; }
 
-#define HasMapField(Src, Key, Fail) if (has_not_map_field(Src, Key)) { Fail; }
-
 #define GetMapElement(Src, Key, Dst, Fail)	\
   do {						\
      Eterm _res = get_map_element(Src, Key);	\
@@ -960,7 +958,6 @@ static Eterm update_map_assoc(Process* p, Eterm* reg,
 			      Eterm map, BeamInstr* I) NOINLINE;
 static Eterm update_map_exact(Process* p, Eterm* reg,
 			      Eterm map, BeamInstr* I) NOINLINE;
-static int has_not_map_field(Eterm map, Eterm key);
 static Eterm get_map_element(Eterm map, Eterm key);
 
 /*
@@ -2391,67 +2388,6 @@ void process_main(void)
      Next(3+Arg(2));
  }
 
- OpCase(i_has_map_fields_fsI): {
-    flatmap_t* mp;
-    Eterm map;
-    Eterm field;
-    Eterm *ks;
-    BeamInstr* fs;
-    Uint sz,n;
-
-    GetArg1(1, map);
-    n  = (Uint)Arg(2);
-    fs = &Arg(3); /* pattern fields */
-
-    /* get term from field? */
-    if (is_hashmap(map)) {
-	Uint32 hx;
-	while(n--) {
-	    field = *fs++;
-	    hx = hashmap_make_hash(field);
-	    if (!erts_hashmap_get(hx,field,map)) {
-		SET_I((BeamInstr *) Arg(0));
-		goto has_map_fields_fail;
-	    }
-	}
-	goto has_map_fields_ok;
-    }
-
-    ASSERT(is_flatmap(map));
-
-    mp = (flatmap_t *)flatmap_val(map);
-    sz = flatmap_get_size(mp);
-
-    if (sz == 0) {
-	SET_I((BeamInstr *) Arg(0));
-	goto has_map_fields_fail;
-    }
-
-    ks = flatmap_get_keys(mp);
-
-    ASSERT(n>0);
-
-    while(sz) {
-	field = (Eterm)*fs;
-	if (EQ(field,*ks)) {
-	    n--;
-	    fs++;
-	    if (n == 0) break;
-	}
-	ks++; sz--;
-    }
-
-    if (n) {
-	SET_I((BeamInstr *) Arg(0));
-	goto has_map_fields_fail;
-    }
-has_map_fields_ok:
-    I += 4 + Arg(2);
-has_map_fields_fail:
-    ASSERT(VALID_INSTR(*I));
-    Goto(*I);
- }
-
 #define PUT_TERM_REG(term, desc)				\
 do {								\
     switch ((desc) & _TAG_IMMED1_MASK) {			\
@@ -6470,38 +6406,6 @@ new_fun(Process* p, Eterm* reg, ErlFunEntry* fe, int num_free)
     return make_fun(funp);
 }
 
-static int has_not_map_field(Eterm map, Eterm key)
-{
-    Uint32 hx;
-    if (is_flatmap(map)) {
-	flatmap_t* mp;
-	Eterm* keys;
-	Uint i;
-	Uint n;
-
-	mp   = (flatmap_t *)flatmap_val(map);
-	keys = flatmap_get_keys(mp);
-	n    = flatmap_get_size(mp);
-	if (is_immed(key)) {
-	    for (i = 0; i < n; i++) {
-		if (keys[i] == key) {
-		    return 0;
-		}
-	    }
-	} else {
-	    for (i = 0; i <  n; i++) {
-		if (EQ(keys[i], key)) {
-		    return 0;
-		}
-	    }
-	}
-	return 1;
-    }
-    ASSERT(is_hashmap(map));
-    hx = hashmap_make_hash(key);
-    return erts_hashmap_get(hx,key,map) ? 0 : 1;
-}
-
 static Eterm get_map_element(Eterm map, Eterm key)
 {
     Uint32 hx;
diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 60f4ab5280..18e1e312ad 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -4107,21 +4107,31 @@ gen_get_map_element(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
 }
 
 static GenOp*
-gen_has_map_field(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
-		    GenOpArg Size, GenOpArg* Rest)
+gen_has_map_fields(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
+		   GenOpArg Size, GenOpArg* Rest)
 {
     GenOp* op;
+    Uint i;
+    Uint n;
 
     ASSERT(Size.type == TAG_u);
+    n = Size.val;
 
     NEW_GENOP(stp, op);
+    GENOP_ARITY(op, 3 + 2*n);
     op->next = NULL;
-    op->op = genop_has_map_field_3;
-    op->arity = 4;
+    op->op = genop_get_map_elements_3;
 
     op->a[0] = Fail;
     op->a[1] = Src;
-    op->a[2] = Rest[0];
+    op->a[2].type = TAG_u;
+    op->a[2].val = 2*n;
+
+    for (i = 0; i < n; i++) {
+	op->a[3+2*i] = Rest[i];
+	op->a[3+2*i+1].type = TAG_x;
+	op->a[3+2*i+1].val = 0;	/* x(0); normally not used */
+    }
     return op;
 }
 
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index a5a89b3990..abaa47217a 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1497,31 +1497,10 @@ is_map f r
 is_map f x
 is_map f y
 
-## Transform has_map_field(s) #{ K1 := _, K2 := _ }
-
-has_map_field/3
-
-has_map_fields Fail Src Size=u==1 Rest=* => gen_has_map_field(Fail,Src,Size,Rest)
-has_map_fields Fail Src Size Rest=* => i_has_map_fields Fail Src Size Rest
-
-i_has_map_fields f s I
-
-has_map_field Fail Src=rxy Key=arxy => i_has_map_field Fail Src Key
-has_map_field Fail Src Key => move Key x | i_has_map_field Fail Src x
-
-%macro: i_has_map_field HasMapField -fail_action
-i_has_map_field f r a
-i_has_map_field f x a
-i_has_map_field f y a
-i_has_map_field f r r
-i_has_map_field f x r
-i_has_map_field f y r
-i_has_map_field f r x
-i_has_map_field f x x
-i_has_map_field f y x
-i_has_map_field f r y
-i_has_map_field f x y
-i_has_map_field f y y
+## Transform has_map_fields #{ K1 := _, K2 := _ } to has_map_elements
+
+has_map_fields Fail Src Size Rest=* => \
+   gen_has_map_fields(Fail, Src, Size, Rest)
 
 ## Transform get_map_elements(s) #{ K1 := V1, K2 := V2 }
 
-- 
cgit v1.2.3


From 529af720ae7b663aa6a1a086b31ba9e3605ff21e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 23 Mar 2015 13:10:20 +0100
Subject: Sort maps keys in the loader

The map instructions require that the keys in the instructions
are sorted (for flatmaps). But that is an implementation detail
that should not exposed outside of the BEAM virtual machine.
Therefore, make the sorting of the keys the responsibility of
the loader and not the compiler.

Also note that the sort order for maps with numeric keys or keys
with numeric components has changed in OTP 18. That means that
code compiled for OTP 17 that operated on maps with map keys
might not work in OTP 18 without the sorting in the loader
(although it is unlikely to be an issue in practice).
---
 erts/emulator/beam/beam_load.c   | 80 ++++++++++++++++++++++++++++++++++++++++
 erts/emulator/beam/ops.tab       | 24 ++++++++----
 erts/emulator/test/map_SUITE.erl |  3 ++
 3 files changed, 100 insertions(+), 7 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 18e1e312ad..1a6c6c16ad 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -4081,6 +4081,86 @@ is_empty_map(LoaderState* stp, GenOpArg Lit)
     return is_flatmap(term) && flatmap_get_size(flatmap_val(term)) == 0;
 }
 
+/*
+ * Pseudo predicate map_key_sort that will sort the Rest operand for
+ * map instructions as a side effect.
+ */
+
+typedef struct SortGenOpArg {
+    Eterm term;			/* Term to use for comparing  */
+    GenOpArg arg;		/* Original data */
+} SortGenOpArg;
+
+static int
+genopargtermcompare(SortGenOpArg* a, SortGenOpArg* b)
+{
+    return CMP_TERM(a->term, b->term);
+}
+
+static int
+map_key_sort(LoaderState* stp, GenOpArg Size, GenOpArg* Rest)
+{
+    SortGenOpArg* t;
+    unsigned size = Size.val;
+    unsigned i;
+
+    if (size == 2) {
+	return 1;		/* Already sorted. */
+    }
+
+
+    t = (SortGenOpArg *) erts_alloc(ERTS_ALC_T_TMP, size*sizeof(SortGenOpArg));
+
+    /*
+     * Copy original data and sort keys to a temporary array.
+     */
+    for (i = 0; i < size; i += 2) {
+	t[i].arg = Rest[i];
+	switch (Rest[i].type) {
+	case TAG_a:
+	    t[i].term = Rest[i].val;
+	    ASSERT(is_atom(t[i].term));
+	    break;
+	case TAG_i:
+	    t[i].term = make_small(Rest[i].val);
+	    break;
+	case TAG_n:
+	    t[i].term = NIL;
+	    break;
+	case TAG_q:
+	    t[i].term = stp->literals[Rest[i].val].term;
+	    break;
+	default:
+	    /*
+	     * Not a literal key. Not allowed. Only a single
+	     * variable key is allowed in each map instruction.
+	     */
+	    erts_free(ERTS_ALC_T_TMP, (void *) t);
+	    return 0;
+	}
+#ifdef DEBUG
+	t[i+1].term = THE_NON_VALUE;
+#endif
+	t[i+1].arg = Rest[i+1];
+    }
+
+    /*
+     * Sort the temporary array.
+     */
+    qsort((void *) t, size / 2, 2 * sizeof(SortGenOpArg),
+	  (int (*)(const void *, const void *)) genopargtermcompare);
+
+    /*
+     * Copy back the sorted, original data.
+     */
+    for (i = 0; i < size; i++) {
+	Rest[i] = t[i].arg;
+    }
+
+    erts_free(ERTS_ALC_T_TMP, (void *) t);
+    return 1;
+}
+
 /*
  * Replace a get_map_elements with one key to an instruction with one
  * element
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index abaa47217a..92d9ccb5eb 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1473,16 +1473,24 @@ apply_last I P
 # Map instructions in R17.
 #
 
-put_map_assoc j Map Dst Live Size Rest=* | is_empty_map(Map) => \
+sorted_put_map_assoc/5
+put_map_assoc F Map Dst Live Size Rest=* | map_key_sort(Size, Rest) => \
+  sorted_put_map_assoc F Map Dst Live Size Rest
+
+sorted_put_map_exact/5
+put_map_exact F Map Dst Live Size Rest=* | map_key_sort(Size, Rest) => \
+  sorted_put_map_exact F Map Dst Live Size Rest
+
+sorted_put_map_assoc j Map Dst Live Size Rest=* | is_empty_map(Map) => \
    new_map Dst Live Size Rest
-put_map_assoc F Src=s Dst Live Size Rest=* => \
+sorted_put_map_assoc F Src=s Dst Live Size Rest=* => \
 	       update_map_assoc F Src Dst Live Size Rest
-put_map_assoc F Src Dst Live Size Rest=* => \
+sorted_put_map_assoc F Src Dst Live Size Rest=* => \
 	       move Src x | update_map_assoc F x Dst Live Size Rest
 
-put_map_exact F Src=s Dst Live Size Rest=* => \
+sorted_put_map_exact F Src=s Dst Live Size Rest=* => \
 	      update_map_exact F Src Dst Live Size Rest
-put_map_exact F Src Dst Live Size Rest=* => \
+sorted_put_map_exact F Src Dst Live Size Rest=* => \
 	      move Src x | update_map_exact F x Dst Live Size Rest
 
 new_map d I I
@@ -1506,8 +1514,10 @@ has_map_fields Fail Src Size Rest=* => \
 
 get_map_element/4
 
-get_map_elements Fail Src=rxy Size=u==2 Rest=* => gen_get_map_element(Fail,Src,Size,Rest)
-get_map_elements Fail Src Size Rest=* => i_get_map_elements Fail Src Size Rest
+get_map_elements Fail Src=rxy Size=u==2 Rest=* => \
+    gen_get_map_element(Fail, Src, Size, Rest)
+get_map_elements Fail Src Size Rest=* | map_key_sort(Size, Rest) => \
+   i_get_map_elements Fail Src Size Rest
 
 i_get_map_elements f s I
 
diff --git a/erts/emulator/test/map_SUITE.erl b/erts/emulator/test/map_SUITE.erl
index 7b68e91dc5..008fa0e11b 100644
--- a/erts/emulator/test/map_SUITE.erl
+++ b/erts/emulator/test/map_SUITE.erl
@@ -164,6 +164,9 @@ t_build_and_match_literals(Config) when is_list(Config) ->
     #{1:=a,2:=b,3:="c","4":="d",<<"5">>:=<<"e">>,{"6",7}:="f",8:=g} =
 	id(#{1=>a,2=>b,3=>"c","4"=>"d",<<"5">>=><<"e">>,{"6",7}=>"f",8=>g}),
 
+    #{[]:=a,42.0:=b,x:={x,y},[a,b]:=list} =
+         id(#{[]=>a,42.0=>b,x=>{x,y},[a,b]=>list}),
+
     #{<<"hi all">> := 1} = id(#{<<"hi",32,"all">> => 1}),
 
     #{a:=X,a:=X=3,b:=4} = id(#{a=>3,b=>4}), % weird but ok =)
-- 
cgit v1.2.3


From 47e1ed4c0681a73c9d6bc8d24ece85dd77957034 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Tue, 24 Mar 2015 15:06:25 +0100
Subject: beam_emu: Slightly optimize update_map_{assoc,exact}

In the update loop for big maps, the E variable is restored for
each turn of the loop. It only needs to be restored if a garbage
collection has been performed.

Also add a new test case that attempts to force several garbage
collections while updating a map, to help us find bugs with
incorrect restoration of the E variable after a garbage collection.
---
 erts/emulator/beam/beam_emu.c    |  9 +++----
 erts/emulator/test/map_SUITE.erl | 58 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 8 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index f25b9f594d..4e64dce95c 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -6572,10 +6572,9 @@ update_map_assoc(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
 		reg[live] = res;
 		erts_garbage_collect(p, 0, reg, live+1);
 		res       = reg[live];
+		E = p->stop;
 	    }
 
-	    E = p->stop;
-
 	    new_p += 2;
 	}
 	return res;
@@ -6781,7 +6780,6 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
 	res = map;
 	E = p->stop;
 	while(n--) {
-	    /* assoc can't fail */
 	    GET_TERM(new_p[0], new_key);
 	    GET_TERM(new_p[1], val);
 	    hx = hashmap_make_hash(new_key);
@@ -6795,10 +6793,9 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
 		reg[live] = res;
 		erts_garbage_collect(p, 0, reg, live+1);
 		res       = reg[live];
+		E = p->stop;
 	    }
 
-	    E = p->stop;
-
 	    new_p += 2;
 	}
 	return res;
@@ -6808,7 +6805,7 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
     num_old = flatmap_get_size(old_mp);
 
     /*
-     * If the old map is empty, create a new map.
+     * If the old map is empty, fail.
      */
 
     if (num_old == 0) {
diff --git a/erts/emulator/test/map_SUITE.erl b/erts/emulator/test/map_SUITE.erl
index 008fa0e11b..72b8ad91ef 100644
--- a/erts/emulator/test/map_SUITE.erl
+++ b/erts/emulator/test/map_SUITE.erl
@@ -78,7 +78,8 @@
 	t_tracing/1,
 
 	%% instruction-level tests
-	t_has_map_fields/1
+	t_has_map_fields/1,
+	y_regs/1
     ]).
 
 -include_lib("stdlib/include/ms_transform.hrl").
@@ -138,7 +139,8 @@ all() -> [
 	t_tracing,
 
 	%% instruction-level tests
-	t_has_map_fields
+	t_has_map_fields,
+	y_regs
     ].
 
 groups() -> [].
@@ -2758,5 +2760,57 @@ has_map_fields_3(#{a:=_,b:=_}) -> true;
 has_map_fields_3(#{[]:=_,42.0:=_}) -> true;
 has_map_fields_3(#{}) -> false.
 
+y_regs(Config) when is_list(Config) ->
+    Val = [length(Config)],
+    Map0 = y_regs_update(#{}, Val),
+    Map2 = y_regs_update(Map0, Val),
+
+    Map3 = maps:from_list([{I,I*I} || I <- lists:seq(1, 100)]),
+    Map4 = y_regs_update(Map3, Val),
+
+    true = is_map(Map2) andalso is_map(Map4),
+
+    ok.
+
+y_regs_update(Map0, Val0) ->
+    Val1 = {t,Val0},
+    K1 = id({key,1}),
+    K2 = id({key,2}),
+    Map1 = Map0#{K1=>K1,
+		 a=>Val0,b=>Val0,c=>Val0,d=>Val0,e=>Val0,
+		 f=>Val0,g=>Val0,h=>Val0,i=>Val0,j=>Val0,
+		 k=>Val0,l=>Val0,m=>Val0,n=>Val0,o=>Val0,
+		 p=>Val0,q=>Val0,r=>Val0,s=>Val0,t=>Val0,
+		 u=>Val0,v=>Val0,w=>Val0,x=>Val0,y=>Val0,
+		 z=>Val0,
+		 aa=>Val0,ab=>Val0,ac=>Val0,ad=>Val0,ae=>Val0,
+		 af=>Val0,ag=>Val0,ah=>Val0,ai=>Val0,aj=>Val0,
+		 ak=>Val0,al=>Val0,am=>Val0,an=>Val0,ao=>Val0,
+		 ap=>Val0,aq=>Val0,ar=>Val0,as=>Val0,at=>Val0,
+		 au=>Val0,av=>Val0,aw=>Val0,ax=>Val0,ay=>Val0,
+		 az=>Val0,
+		 K2=>[a,b,c]},
+    Map2 = Map1#{K1=>K1,
+		 a:=Val1,b:=Val1,c:=Val1,d:=Val1,e:=Val1,
+		 f:=Val1,g:=Val1,h:=Val1,i:=Val1,j:=Val1,
+		 k:=Val1,l:=Val1,m:=Val1,n:=Val1,o:=Val1,
+		 p:=Val1,q:=Val1,r:=Val1,s:=Val1,t:=Val1,
+		 u:=Val1,v:=Val1,w:=Val1,x:=Val1,y:=Val1,
+		 z:=Val1,
+		 aa:=Val1,ab:=Val1,ac:=Val1,ad:=Val1,ae:=Val1,
+		 af:=Val1,ag:=Val1,ah:=Val1,ai:=Val1,aj:=Val1,
+		 ak:=Val1,al:=Val1,am:=Val1,an:=Val1,ao:=Val1,
+		 ap:=Val1,aq:=Val1,ar:=Val1,as:=Val1,at:=Val1,
+		 au:=Val1,av:=Val1,aw:=Val1,ax:=Val1,ay:=Val1,
+		 az:=Val1,
+		 K2=>[a,b,c]},
+
+    %% Traverse the maps to validate them.
+    _ = erlang:phash2({Map1,Map2}, 100000),
+
+    _ = id({K1,K2,Val0,Val1}),			%Force use of Y registers.
+    Map2.
+
+
 %% Use this function to avoid compile-time evaluation of an expression.
 id(I) -> I.
-- 
cgit v1.2.3


From 5b8872c37f63b35e13464109e986ef3727588040 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Wed, 25 Mar 2015 09:47:42 +0100
Subject: Optimize use of i_get_map_element/4

In the i_get_map_element/4 instruction, for literal keys other than
atoms, the key would be put into x[0] instead of used directly in the
instruction. The reason is that the original implementation of maps
only supported atom keys.
---
 erts/emulator/beam/ops.tab | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 92d9ccb5eb..23f5b75b7a 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1521,21 +1521,21 @@ get_map_elements Fail Src Size Rest=* | map_key_sort(Size, Rest) => \
 
 i_get_map_elements f s I
 
-get_map_element Fail Src=rxy Key=ax Dst => i_get_map_element Fail Src Key Dst
-get_map_element Fail Src=rxy Key=rycq Dst => \
+get_map_element Fail Src=rxy Key=cx Dst => i_get_map_element Fail Src Key Dst
+get_map_element Fail Src=rxy Key=ry Dst => \
     move Key x | i_get_map_element Fail Src x Dst
 get_map_element Fail Src Key Dst => jump Fail
 
 %macro: i_get_map_element GetMapElement -fail_action
-i_get_map_element f r a r
-i_get_map_element f x a r
-i_get_map_element f y a r
-i_get_map_element f r a x
-i_get_map_element f x a x
-i_get_map_element f y a x
-i_get_map_element f r a y
-i_get_map_element f x a y
-i_get_map_element f y a y
+i_get_map_element f r c r
+i_get_map_element f x c r
+i_get_map_element f y c r
+i_get_map_element f r c x
+i_get_map_element f x c x
+i_get_map_element f y c x
+i_get_map_element f r c y
+i_get_map_element f x c y
+i_get_map_element f y c y
 i_get_map_element f r x r
 i_get_map_element f x x r
 i_get_map_element f y x r
-- 
cgit v1.2.3


From 5f1e301dfec48fccbf865a8b54af5908bebb77c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Wed, 25 Mar 2015 11:57:44 +0100
Subject: Teach the loader to pre-compute the hash value for single-key lookups

Let the loader pre-compute the hash value when a single, literal key
is matched as in:

  #{<<"some_key">>:=V} = Map

In my measurements, this optimization resulted in a 30 percent
speedup for short binary keys.

Unfortunately, this optimizization makes no difference for small
maps with less than 32 keys, since the hash value is not used.
Still, there are the following use cases:

* A map used instead of a record with more than 32 entries. I have
seen some applications with huge records.

* Lookup in JSON dictionaries represented as maps.
The hash value will only be used when the map is a hash map
(currently, that means at least 32 entries).
---
 erts/emulator/beam/beam_emu.c  | 46 ++++++++++++++++++++++++++++++++++++++++++
 erts/emulator/beam/beam_load.c | 42 +++++++++++++++++++++++++++++++++-----
 erts/emulator/beam/ops.tab     | 26 +++++++++++-------------
 3 files changed, 95 insertions(+), 19 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 4e64dce95c..5f49032e23 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -710,6 +710,15 @@ void** beam_ops;
      Dst = _res;				\
   } while (0)
 
+#define GetMapElementHash(Src, Key, Hx, Dst, Fail)	\
+  do {							\
+     Eterm _res = get_map_element_hash(Src, Key, Hx);	\
+     if (is_non_value(_res)) {				\
+        Fail;						\
+     }							\
+     Dst = _res;					\
+  } while (0)
+
 #define IsFunction(X, Action)			\
   do {						\
      if ( !(is_any_fun(X)) ) {			\
@@ -959,6 +968,7 @@ static Eterm update_map_assoc(Process* p, Eterm* reg,
 static Eterm update_map_exact(Process* p, Eterm* reg,
 			      Eterm map, BeamInstr* I) NOINLINE;
 static Eterm get_map_element(Eterm map, Eterm key);
+static Eterm get_map_element_hash(Eterm map, Eterm key, Uint32 hx);
 
 /*
  * Functions not directly called by process_main(). OK to inline.
@@ -6441,6 +6451,42 @@ static Eterm get_map_element(Eterm map, Eterm key)
     return vs ? *vs : THE_NON_VALUE;
 }
 
+static Eterm get_map_element_hash(Eterm map, Eterm key, Uint32 hx)
+{
+    const Eterm *vs;
+
+    if (is_flatmap(map)) {
+	flatmap_t *mp;
+	Eterm *ks;
+	Uint i;
+	Uint n;
+
+	mp = (flatmap_t *)flatmap_val(map);
+	ks = flatmap_get_keys(mp);
+	vs = flatmap_get_values(mp);
+	n  = flatmap_get_size(mp);
+	if (is_immed(key)) {
+	    for (i = 0; i < n; i++) {
+		if (ks[i] == key) {
+		    return vs[i];
+		}
+	    }
+	} else {
+	    for (i = 0; i < n; i++) {
+		if (EQ(ks[i], key)) {
+		    return vs[i];
+		}
+	    }
+	}
+	return THE_NON_VALUE;
+    }
+
+    ASSERT(is_hashmap(map));
+    ASSERT(hx == hashmap_make_hash(key));
+    vs = erts_hashmap_get(hx, key, map);
+    return vs ? *vs : THE_NON_VALUE;
+}
+
 #define GET_TERM(term, dest)					\
 do {								\
     Eterm src = (Eterm)(term);					\
diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 1a6c6c16ad..ee319d0a78 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -4161,9 +4161,30 @@ map_key_sort(LoaderState* stp, GenOpArg Size, GenOpArg* Rest)
     return 1;
 }
 
+static int
+hash_genop_arg(LoaderState* stp, GenOpArg Key, Uint32* hx)
+{
+    switch (Key.type) {
+    case TAG_a:
+	*hx = hashmap_make_hash(Key.val);
+	return 1;
+    case TAG_i:
+	*hx = hashmap_make_hash(make_small(Key.val));
+	return 1;
+    case TAG_n:
+	*hx = hashmap_make_hash(NIL);
+	return 1;
+    case TAG_q:
+	*hx = hashmap_make_hash(stp->literals[Key.val].term);
+	return 1;
+    default:
+	return 0;
+    }
+}
+
 /*
  * Replace a get_map_elements with one key to an instruction with one
- * element
+ * element.
  */
 
 static GenOp*
@@ -4171,18 +4192,29 @@ gen_get_map_element(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
 		    GenOpArg Size, GenOpArg* Rest)
 {
     GenOp* op;
+    GenOpArg Key;
+    Uint32 hx = 0;
 
     ASSERT(Size.type == TAG_u);
 
     NEW_GENOP(stp, op);
     op->next = NULL;
-    op->op = genop_get_map_element_4;
-    op->arity = 4;
-
     op->a[0] = Fail;
     op->a[1] = Src;
     op->a[2] = Rest[0];
-    op->a[3] = Rest[1];
+
+    Key = Rest[0];
+    if (hash_genop_arg(stp, Key, &hx)) {
+	op->arity = 5;
+	op->op = genop_i_get_map_element_hash_5;
+	op->a[3].type = TAG_u;
+	op->a[3].val = (BeamInstr) hx;
+	op->a[4] = Rest[1];
+    } else {
+	op->arity = 4;
+	op->op = genop_i_get_map_element_4;
+	op->a[3] = Rest[1];
+    }
     return op;
 }
 
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 23f5b75b7a..456f879ab5 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1512,8 +1512,6 @@ has_map_fields Fail Src Size Rest=* => \
 
 ## Transform get_map_elements(s) #{ K1 := V1, K2 := V2 }
 
-get_map_element/4
-
 get_map_elements Fail Src=rxy Size=u==2 Rest=* => \
     gen_get_map_element(Fail, Src, Size, Rest)
 get_map_elements Fail Src Size Rest=* | map_key_sort(Size, Rest) => \
@@ -1521,21 +1519,21 @@ get_map_elements Fail Src Size Rest=* | map_key_sort(Size, Rest) => \
 
 i_get_map_elements f s I
 
-get_map_element Fail Src=rxy Key=cx Dst => i_get_map_element Fail Src Key Dst
-get_map_element Fail Src=rxy Key=ry Dst => \
+i_get_map_element Fail Src=rxy Key=ry Dst => \
     move Key x | i_get_map_element Fail Src x Dst
-get_map_element Fail Src Key Dst => jump Fail
+
+%macro: i_get_map_element_hash GetMapElementHash -fail_action
+i_get_map_element_hash f r c I r
+i_get_map_element_hash f x c I r
+i_get_map_element_hash f y c I r
+i_get_map_element_hash f r c I x
+i_get_map_element_hash f x c I x
+i_get_map_element_hash f y c I x
+i_get_map_element_hash f r c I y
+i_get_map_element_hash f x c I y
+i_get_map_element_hash f y c I y
 
 %macro: i_get_map_element GetMapElement -fail_action
-i_get_map_element f r c r
-i_get_map_element f x c r
-i_get_map_element f y c r
-i_get_map_element f r c x
-i_get_map_element f x c x
-i_get_map_element f y c x
-i_get_map_element f r c y
-i_get_map_element f x c y
-i_get_map_element f y c y
 i_get_map_element f r x r
 i_get_map_element f x x r
 i_get_map_element f y x r
-- 
cgit v1.2.3


From 9b940b12210500e615ea05b447b0e1be34e70d39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Wed, 25 Mar 2015 13:10:30 +0100
Subject: Pre-compute hash values for the general get_map_elements instruction

See the previous commit for justification and use cases.
---
 erts/emulator/beam/beam_emu.c  | 14 +++++++-------
 erts/emulator/beam/beam_load.c | 41 +++++++++++++++++++++++++++++++++++++++++
 erts/emulator/beam/ops.tab     |  2 +-
 3 files changed, 49 insertions(+), 8 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 5f49032e23..6fde14bc8d 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -2427,7 +2427,7 @@ do {								\
      * i.e. that it follows a test is_map if needed.
      */
 
-    n  = (Uint)Arg(2) / 2;
+    n  = (Uint)Arg(2) / 3;
     fs = &Arg(3); /* pattern fields and target registers */
 
     if (is_flatmap(map)) {
@@ -2450,12 +2450,11 @@ do {								\
 	    if (EQ((Eterm)*fs,*ks)) {
 		PUT_TERM_REG(*vs, fs[1]);
 		n--;
-		fs += 2;
+		fs += 3;
 		/* no more values to fetch, we are done */
 		if (n == 0) break;
 	    }
-	    ks++; sz--;
-	    vs++;
+	    ks++, sz--, vs++;
 	}
 
 	if (n) {
@@ -2467,13 +2466,14 @@ do {								\
 	Uint32 hx;
 	ASSERT(is_hashmap(map));
 	while(n--) {
-	    hx = hashmap_make_hash((Eterm)*fs);
-	    if ((v = erts_hashmap_get(hx,(Eterm)*fs, map)) == NULL) {
+	    hx = fs[2];
+	    ASSERT(hx == hashmap_make_hash((Eterm)fs[0]));
+	    if ((v = erts_hashmap_get(hx, (Eterm)fs[0], map)) == NULL) {
 		SET_I((BeamInstr *) Arg(0));
 		goto get_map_elements_fail;
 	    }
 	    PUT_TERM_REG(*v, fs[1]);
-	    fs += 2;
+	    fs += 3;
 	}
     }
 
diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index ee319d0a78..8d7beb4eb4 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -4218,6 +4218,47 @@ gen_get_map_element(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
     return op;
 }
 
+static GenOp*
+gen_get_map_elements(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
+		     GenOpArg Size, GenOpArg* Rest)
+{
+    GenOp* op;
+    Uint32 hx;
+    Uint i;
+    GenOpArg* dst;
+#ifdef DEBUG
+    int good_hash;
+#endif
+
+    ASSERT(Size.type == TAG_u);
+
+    NEW_GENOP(stp, op);
+    op->op = genop_i_get_map_elements_3;
+    GENOP_ARITY(op, 3 + 3*(Size.val/2));
+    op->next = NULL;
+    op->a[0] = Fail;
+    op->a[1] = Src;
+    op->a[2].type = TAG_u;
+    op->a[2].val = 3*(Size.val/2);
+
+    dst = op->a+3;
+    for (i = 0; i < Size.val / 2; i++) {
+	dst[0] = Rest[2*i];
+	dst[1] = Rest[2*i+1];
+#ifdef DEBUG
+	good_hash =
+#endif
+	    hash_genop_arg(stp, dst[0], &hx);
+#ifdef DEBUG
+	ASSERT(good_hash);
+#endif
+	dst[2].type = TAG_u;
+	dst[2].val = (BeamInstr) hx;
+	dst += 3;
+    }
+    return op;
+}
+
 static GenOp*
 gen_has_map_fields(LoaderState* stp, GenOpArg Fail, GenOpArg Src,
 		   GenOpArg Size, GenOpArg* Rest)
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 456f879ab5..9bdc9cb88d 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1515,7 +1515,7 @@ has_map_fields Fail Src Size Rest=* => \
 get_map_elements Fail Src=rxy Size=u==2 Rest=* => \
     gen_get_map_element(Fail, Src, Size, Rest)
 get_map_elements Fail Src Size Rest=* | map_key_sort(Size, Rest) => \
-   i_get_map_elements Fail Src Size Rest
+   gen_get_map_elements(Fail, Src, Size, Rest)
 
 i_get_map_elements f s I
 
-- 
cgit v1.2.3


From 61712dda57647804aec8d4f45da8ae1ccfad732c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 26 Mar 2015 10:59:16 +0100
Subject: Tigthen code for the i_get_map_elements/3 instruction

---
 erts/emulator/beam/beam_emu.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 6fde14bc8d..7e242640ed 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -2439,28 +2439,27 @@ do {								\
 	sz = flatmap_get_size(mp);
 
 	if (sz == 0) {
-	    SET_I((BeamInstr *) Arg(0));
-	    goto get_map_elements_fail;
+	    ClauseFail();
 	}
 
 	ks = flatmap_get_keys(mp);
 	vs = flatmap_get_values(mp);
 
 	while(sz) {
-	    if (EQ((Eterm)*fs,*ks)) {
+	    if (EQ((Eterm) fs[0], *ks)) {
 		PUT_TERM_REG(*vs, fs[1]);
 		n--;
 		fs += 3;
 		/* no more values to fetch, we are done */
-		if (n == 0) break;
+		if (n == 0) {
+		    I = fs;
+		    Next(-1);
+		}
 	    }
 	    ks++, sz--, vs++;
 	}
 
-	if (n) {
-	    SET_I((BeamInstr *) Arg(0));
-	    goto get_map_elements_fail;
-	}
+	ClauseFail();
     } else {
 	const Eterm *v;
 	Uint32 hx;
@@ -2469,19 +2468,14 @@ do {								\
 	    hx = fs[2];
 	    ASSERT(hx == hashmap_make_hash((Eterm)fs[0]));
 	    if ((v = erts_hashmap_get(hx, (Eterm)fs[0], map)) == NULL) {
-		SET_I((BeamInstr *) Arg(0));
-		goto get_map_elements_fail;
+		ClauseFail();
 	    }
 	    PUT_TERM_REG(*v, fs[1]);
 	    fs += 3;
 	}
+	I = fs;
+	Next(-1);
     }
-
-
-    I += 4 + Arg(2);
-get_map_elements_fail:
-    ASSERT(VALID_INSTR(*I));
-    Goto(*I);
  }
 #undef PUT_TERM_REG
 
-- 
cgit v1.2.3


From 44c5a955c8409cc98a5fbdec7898de46b9e5dfbe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Tue, 31 Mar 2015 10:15:33 +0200
Subject: erl_term.h: Add is_not_map() macro

For consistency with other data types, add the is_not_map() macro.
---
 erts/emulator/beam/erl_term.h | 1 +
 1 file changed, 1 insertion(+)

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/erl_term.h b/erts/emulator/beam/erl_term.h
index cff012d5d1..602aab46dc 100644
--- a/erts/emulator/beam/erl_term.h
+++ b/erts/emulator/beam/erl_term.h
@@ -1027,6 +1027,7 @@ _ET_DECLARE_CHECKED(struct erl_node_*,external_ref_node,Eterm)
 
 #define is_map_header(x)       (((x) & (_TAG_HEADER_MASK)) == _TAG_HEADER_MAP)
 #define is_map(x)              (is_boxed((x)) && is_map_header(*boxed_val(x)))
+#define is_not_map(x)          (!is_map(x))
 #define is_map_rel(RTERM,BASE) is_map(rterm2wterm(RTERM,BASE))
 
 /* number tests */
-- 
cgit v1.2.3


From 5bb8262bd4ae8dbcc7438e80de5cedac7ccee1af Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Tue, 31 Mar 2015 08:47:57 +0200
Subject: Raise more descriptive error messages for failed map operations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

According to EEP-43 for maps, a 'badmap' exception should be
generated when an attempt is made to update non-map term such as:

   <<>>#{a=>42}

That was not implemented in the OTP 17.

José Valim suggested that we should take the opportunity to
improve the errors coming from map operations:

  http://erlang.org/pipermail/erlang-questions/2015-February/083588.html

This commit implement better errors from map operations similar
to his suggestion.

When a map update operation (Map#{...}) or a BIF that expects a map
is given a non-map term, the exception will be:

  {badmap,Term}

This kind of exception is similar to the {badfun,Term} exception
from operations that expect a fun.

When a map operation requires a key that is not present in a map,
the following exception will be raised:

  {badkey,Key}

José Valim suggested that the exception should be
{badkey,Key,Map}. We decided not to do that because the map
could potentially be huge and cause problems if the error
propagated through links to other processes.

For BIFs, it could be argued that the exceptions could be simply
'badmap' and 'badkey', because the bad map and bad key can be found in
the argument list for the BIF in the stack backtrace. However, for the
map update operation (Map#{...}), the bad map or bad key will not be
included in the stack backtrace, so that information must be included
in the exception reason itself. For consistency, the BIFs should raise
the same exceptions as update operation.

If more than one key is missing, it is undefined which of
keys that will be reported in the {badkey,Key} exception.
---
 erts/emulator/beam/atom.names                    |   2 +-
 erts/emulator/beam/beam_emu.c                    |  31 ++++-
 erts/emulator/beam/erl_bif_guard.c               |   3 +-
 erts/emulator/beam/erl_map.c                     |  75 +++++++----
 erts/emulator/beam/error.h                       |  10 +-
 erts/emulator/test/map_SUITE.erl                 | 162 ++++++++++++++---------
 erts/emulator/test/map_SUITE_data/badmap_17.beam | Bin 0 -> 592 bytes
 erts/emulator/test/map_SUITE_data/badmap_17.erl  |  26 ++++
 8 files changed, 215 insertions(+), 94 deletions(-)
 create mode 100644 erts/emulator/test/map_SUITE_data/badmap_17.beam
 create mode 100644 erts/emulator/test/map_SUITE_data/badmap_17.erl

(limited to 'erts/emulator')

diff --git a/erts/emulator/beam/atom.names b/erts/emulator/beam/atom.names
index ae3f30d82f..8fdcbb4058 100644
--- a/erts/emulator/beam/atom.names
+++ b/erts/emulator/beam/atom.names
@@ -104,7 +104,7 @@ atom await_sched_wall_time_modifications
 atom awaiting_load
 atom awaiting_unload
 atom backtrace backtrace_depth
-atom badarg badarith badarity badfile badmatch badsig badfun
+atom badarg badarith badarity badfile badfun badkey badmap badmatch badsig
 atom bag
 atom band
 atom big
diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 7e242640ed..6a5128e7f8 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -2493,7 +2493,13 @@ do {								\
 	 StoreResult(res, Arg(2));
 	 Next(5+Arg(4));
      } else {
-	 goto badarg;
+	 /*
+	  * This can only happen if the code was compiled
+	  * with the compiler in OTP 17.
+	  */
+	 c_p->freason = BADMAP;
+	 c_p->fvalue = map;
+	 goto lb_Cl_error;
      }
  }
 
@@ -2511,7 +2517,7 @@ do {								\
 	 StoreResult(res, Arg(2));
 	 Next(5+Arg(4));
      } else {
-	 goto badarg;
+	 goto lb_Cl_error;
      }
  }
 
@@ -5261,7 +5267,9 @@ Eterm error_atom[NUMBER_EXIT_CODES] = {
   am_notalive,		/* 14 */
   am_system_limit,	/* 15 */
   am_try_clause,	/* 16 */
-  am_notsup		/* 17 */
+  am_notsup,		/* 17 */
+  am_badmap,		/* 18 */
+  am_badkey,		/* 19 */
 };
 
 /*
@@ -5517,6 +5525,8 @@ expand_error_value(Process* c_p, Uint freason, Eterm Value) {
     case (GET_EXC_INDEX(EXC_TRY_CLAUSE)):
     case (GET_EXC_INDEX(EXC_BADFUN)):
     case (GET_EXC_INDEX(EXC_BADARITY)):
+    case (GET_EXC_INDEX(EXC_BADMAP)):
+    case (GET_EXC_INDEX(EXC_BADKEY)):
         /* Some common exceptions: value -> {atom, value} */
         ASSERT(is_value(Value));
 	hp = HAlloc(c_p, 3);
@@ -6814,8 +6824,11 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
 	/* apparently the compiler does not emit is_map instructions,
 	 * bad compiler */
 
-	if (is_not_hashmap(map))
+	if (is_not_hashmap(map)) {
+	    p->freason = BADMAP;
+	    p->fvalue = map;
 	    return THE_NON_VALUE;
+	}
 
 	res = map;
 	E = p->stop;
@@ -6825,8 +6838,11 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
 	    hx = hashmap_make_hash(new_key);
 
 	    res = erts_hashmap_insert(p, hx, new_key, val, res,  1);
-	    if (is_non_value(res))
+	    if (is_non_value(res)) {
+		p->fvalue = new_key;
+		p->freason = BADKEY;
 		return res;
+	    }
 
 	    if (p->mbuf) {
 		Uint live = Arg(3);
@@ -6849,6 +6865,9 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
      */
 
     if (num_old == 0) {
+	E = p->stop;
+	p->freason = BADKEY;
+	GET_TERM(new_p[0], p->fvalue);
 	return THE_NON_VALUE;
     }
 
@@ -6918,6 +6937,8 @@ update_map_exact(Process* p, Eterm* reg, Eterm map, BeamInstr* I)
      * update list did not previously exist.
      */
     ASSERT(hp == p->htop + need);
+    p->freason = BADKEY;
+    p->fvalue = new_key;
     return THE_NON_VALUE;
 }
 #undef GET_TERM
diff --git a/erts/emulator/beam/erl_bif_guard.c b/erts/emulator/beam/erl_bif_guard.c
index e7d84ebda1..069327ee9d 100644
--- a/erts/emulator/beam/erl_bif_guard.c
+++ b/erts/emulator/beam/erl_bif_guard.c
@@ -467,7 +467,8 @@ Eterm erts_gc_map_size_1(Process* p, Eterm* reg, Uint live)
     } else if (is_hashmap(arg)) {
 	size = hashmap_size(arg);
     } else {
-	BIF_ERROR(p, BADARG);
+	p->fvalue = arg;
+	BIF_ERROR(p, BADMAP);
     }
     if (IS_USMALL(0, size)) {
 	return make_small(size);
diff --git a/erts/emulator/beam/erl_map.c b/erts/emulator/beam/erl_map.c
index 98023bbb47..3aebbfdaa3 100644
--- a/erts/emulator/beam/erl_map.c
+++ b/erts/emulator/beam/erl_map.c
@@ -122,7 +122,8 @@ BIF_RETTYPE map_size_1(BIF_ALIST_1) {
 	BIF_RET(res);
     }
 
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_1;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /* maps:to_list/1 */
@@ -150,7 +151,8 @@ BIF_RETTYPE maps_to_list_1(BIF_ALIST_1) {
 	return hashmap_to_list(BIF_P, BIF_ARG_1);
     }
 
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_1;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /* maps:find/2
@@ -217,34 +219,29 @@ BIF_RETTYPE maps_find_2(BIF_ALIST_2) {
 	}
 	BIF_RET(am_error);
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_2;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /* maps:get/2
  * return value if key *matches* a key in the map
- * exception bad_key if none matches
+ * exception badkey if none matches
  */
 
 BIF_RETTYPE maps_get_2(BIF_ALIST_2) {
     if (is_map(BIF_ARG_2)) {
-	Eterm *hp;
-        Eterm error;
         const Eterm *value;
-	char *s_error;
 
         value = erts_maps_get(BIF_ARG_1, BIF_ARG_2);
         if (value) {
             BIF_RET(*value);
 	}
 
-	s_error = "bad_key";
-	error = am_atom_put(s_error, sys_strlen(s_error));
-
-	hp = HAlloc(BIF_P, 3);
-	BIF_P->fvalue = TUPLE2(hp, error, BIF_ARG_1);
-	BIF_ERROR(BIF_P, EXC_ERROR_2);
+	BIF_P->fvalue = BIF_ARG_1;
+	BIF_ERROR(BIF_P, BADKEY);
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_2;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /* maps:from_list/1
@@ -911,7 +908,8 @@ BIF_RETTYPE maps_is_key_2(BIF_ALIST_2) {
     if (is_map(BIF_ARG_2)) {
 	BIF_RET(erts_maps_get(BIF_ARG_1, BIF_ARG_2) ? am_true : am_false);
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_2;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /* maps:keys/1 */
@@ -939,7 +937,8 @@ BIF_RETTYPE maps_keys_1(BIF_ALIST_1) {
     } else if (is_hashmap(BIF_ARG_1)) {
 	BIF_RET(hashmap_keys(BIF_P, BIF_ARG_1));
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_1;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 /* maps:merge/2 */
 
@@ -951,6 +950,7 @@ BIF_RETTYPE maps_merge_2(BIF_ALIST_2) {
 	    /* Will always become a tree */
 	    BIF_RET(map_merge_mixed(BIF_P, BIF_ARG_1, BIF_ARG_2, 0));
 	}
+	BIF_P->fvalue = BIF_ARG_2;
     } else if (is_hashmap(BIF_ARG_1)) {
 	if (is_hashmap(BIF_ARG_2)) {
 	    BIF_RET(hashmap_merge(BIF_P, BIF_ARG_1, BIF_ARG_2));
@@ -958,8 +958,11 @@ BIF_RETTYPE maps_merge_2(BIF_ALIST_2) {
 	    /* Will always become a tree */
 	    BIF_RET(map_merge_mixed(BIF_P, BIF_ARG_2, BIF_ARG_1, 1));
 	}
+	BIF_P->fvalue = BIF_ARG_2;
+    } else {
+	BIF_P->fvalue = BIF_ARG_1;
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 static Eterm flatmap_merge(Process *p, Eterm nodeA, Eterm nodeB) {
@@ -1398,7 +1401,8 @@ BIF_RETTYPE maps_put_3(BIF_ALIST_3) {
     if (is_map(BIF_ARG_3)) {
 	BIF_RET(erts_maps_put(BIF_P, BIF_ARG_1, BIF_ARG_2, BIF_ARG_3));
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_3;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /* maps:remove/3 */
@@ -1492,7 +1496,8 @@ BIF_RETTYPE maps_remove_2(BIF_ALIST_2) {
 	    BIF_RET(res);
 	}
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_2;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 int erts_maps_update(Process *p, Eterm key, Eterm value, Eterm map, Eterm *res) {
@@ -1688,13 +1693,17 @@ Eterm erts_maps_put(Process *p, Eterm key, Eterm value, Eterm map) {
 /* maps:update/3 */
 
 BIF_RETTYPE maps_update_3(BIF_ALIST_3) {
-    if (is_map(BIF_ARG_3)) {
+    if (is_not_map(BIF_ARG_3)) {
+	BIF_P->fvalue = BIF_ARG_3;
+	BIF_ERROR(BIF_P, BADMAP);
+    } else {
 	Eterm res;
 	if (erts_maps_update(BIF_P, BIF_ARG_1, BIF_ARG_2, BIF_ARG_3, &res)) {
 	    BIF_RET(res);
 	}
+	BIF_P->fvalue = BIF_ARG_1;
+	BIF_ERROR(BIF_P, BADKEY);
     }
-    BIF_ERROR(BIF_P, BADARG);
 }
 
 
@@ -1723,7 +1732,8 @@ BIF_RETTYPE maps_values_1(BIF_ALIST_1) {
     } else if (is_hashmap(BIF_ARG_1)) {
 	BIF_RET(hashmap_values(BIF_P, BIF_ARG_1));
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_1;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 static Eterm hashmap_to_list(Process *p, Eterm node) {
@@ -2546,8 +2556,12 @@ Uint hashmap_over_estimated_heap_size(Uint k)
 BIF_RETTYPE erts_debug_map_info_1(BIF_ALIST_1) {
     if (is_hashmap(BIF_ARG_1)) {
 	BIF_RET(hashmap_info(BIF_P,BIF_ARG_1));
+    } else if (is_flatmap(BIF_ARG_1)) {
+	BIF_ERROR(BIF_P, BADARG);
+    } else {
+	BIF_P->fvalue = BIF_ARG_1;
+	BIF_ERROR(BIF_P, BADMAP);
     }
-    BIF_ERROR(BIF_P, BADARG);
 }
 
 /*
@@ -2560,8 +2574,12 @@ BIF_RETTYPE erts_internal_map_to_tuple_keys_1(BIF_ALIST_1) {
     if (is_flatmap(BIF_ARG_1)) {
 	flatmap_t *mp = (flatmap_t*)flatmap_val(BIF_ARG_1);
 	BIF_RET(mp->keys);
+    } else if (is_hashmap(BIF_ARG_1)) {
+	BIF_ERROR(BIF_P, BADARG);
+    } else {
+	BIF_P->fvalue = BIF_ARG_1;
+	BIF_ERROR(BIF_P, BADMAP);
     }
-    BIF_ERROR(BIF_P, BADARG);
 }
 
 /*
@@ -2589,7 +2607,8 @@ BIF_RETTYPE erts_internal_map_type_1(BIF_ALIST_1) {
                 erl_exit(1, "bad header");
         }
     }
-    BIF_ERROR(BIF_P, BADARG);
+    BIF_P->fvalue = BIF_ARG_1;
+    BIF_ERROR(BIF_P, BADMAP);
 }
 
 /*
@@ -2629,8 +2648,12 @@ BIF_RETTYPE erts_internal_map_hashmap_children_1(BIF_ALIST_1) {
         hp = HAlloc(BIF_P, 2*sz);
         while(sz--) { res = CONS(hp, *ptr++, res); hp += 2; }
         BIF_RET(res);
+    } else if (is_flatmap(BIF_ARG_1)) {
+	BIF_ERROR(BIF_P, BADARG);
+    } else {
+	BIF_P->fvalue = BIF_ARG_1;
+	BIF_ERROR(BIF_P, BADMAP);
     }
-    BIF_ERROR(BIF_P, BADARG);
 }
 
 
diff --git a/erts/emulator/beam/error.h b/erts/emulator/beam/error.h
index ddc2c1396d..e63967adb6 100644
--- a/erts/emulator/beam/error.h
+++ b/erts/emulator/beam/error.h
@@ -140,7 +140,13 @@
 #define EXC_NOTSUP		((17 << 8) | EXC_ERROR)
 					/* Not supported */
 
-#define NUMBER_EXIT_CODES 18	/* The number of exit code indices */
+#define EXC_BADMAP		((18 << 8) | EXC_ERROR)
+					/* Bad map */
+
+#define EXC_BADKEY		((19 << 8) | EXC_ERROR)
+					/* Bad key in map */
+
+#define NUMBER_EXIT_CODES 20	/* The number of exit code indices */
 
 /*
  * Internal pseudo-error codes.
@@ -152,6 +158,8 @@
  */
 #define BADARG EXC_BADARG
 #define BADARITH EXC_BADARITH
+#define BADKEY EXC_BADKEY
+#define BADMAP EXC_BADMAP
 #define BADMATCH EXC_BADMATCH
 #define SYSTEM_LIMIT EXC_SYSTEM_LIMIT
 
diff --git a/erts/emulator/test/map_SUITE.erl b/erts/emulator/test/map_SUITE.erl
index 72b8ad91ef..39549282c0 100644
--- a/erts/emulator/test/map_SUITE.erl
+++ b/erts/emulator/test/map_SUITE.erl
@@ -79,7 +79,8 @@
 
 	%% instruction-level tests
 	t_has_map_fields/1,
-	y_regs/1
+	y_regs/1,
+	badmap_17/1
     ]).
 
 -include_lib("stdlib/include/ms_transform.hrl").
@@ -140,7 +141,8 @@ all() -> [
 
 	%% instruction-level tests
 	t_has_map_fields,
-	y_regs
+	y_regs,
+	badmap_17
     ].
 
 groups() -> [].
@@ -676,9 +678,10 @@ t_map_size(Config) when is_list(Config) ->
     N  = map_size(maps:from_list([{float(I),I}||I<-Is])),
 
     %% Error cases.
-    {'EXIT',{badarg,_}} = (catch map_size([])),
-    {'EXIT',{badarg,_}} = (catch map_size(<<1,2,3>>)),
-    {'EXIT',{badarg,_}} = (catch map_size(1)),
+    do_badmap(fun(T) ->
+                    {'EXIT',{{badmap,T},_}} =
+                      (catch map_size(T))
+              end),
     ok.
 
 build_and_check_size([K|Ks],N,M0) ->
@@ -878,9 +881,11 @@ t_update_map_expressions(Config) when is_list(Config) ->
     #{ "aa" := {$a,$a}, "ac":=41, "dc":=42 } =
         (maps:from_list([{[K1,K2],{K1,K2}}|| K1 <- Ks, K2 <- Ks]))#{ "ac" := 41, "dc" => 42 },
 
-    %% Error cases, FIXME: should be 'badmap'?
-    {'EXIT',{badarg,_}} = (catch (id(<<>>))#{ a := 42, b => 2 }),
-    {'EXIT',{badarg,_}} = (catch (id([]))#{ a := 42, b => 2 }),
+    %% Error cases.
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},_}} =
+			  (catch (T)#{a:=42,b=>2})
+	      end),
     ok.
 
 t_update_assoc(Config) when is_list(Config) ->
@@ -895,8 +900,10 @@ t_update_assoc(Config) when is_list(Config) ->
     M2 = M0#{3.0:=wrong,3.0=>new},
 
     %% Errors cases.
-    BadMap = id(badmap),
-    {'EXIT',{badarg,_}} = (catch BadMap#{nonexisting=>val}),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},_}} =
+			  (catch T#{nonexisting=>val})
+	      end),
     ok.
 
 
@@ -963,9 +970,6 @@ t_update_assoc_large(Config) when is_list(Config) ->
     #{10:=a0,20:=b0,13.0:=new,"40":="d0",<<"50">>:="e0"} = M2,
     M2 = M0#{13.0:=wrong,13.0=>new},
 
-    %% Errors cases.
-    BadMap = id(badmap),
-    {'EXIT',{badarg,_}} = (catch BadMap#{nonexisting=>M0}),
     ok.
 
 t_update_exact(Config) when is_list(Config) ->
@@ -987,12 +991,17 @@ t_update_exact(Config) when is_list(Config) ->
 	1 := update2, 1.0 := new_val2, 1.0 => new_val3,
 	1.0 => new_val4 },
 
-
     %% Errors cases.
-    {'EXIT',{badarg,_}} = (catch M0#{nonexisting:=val}),
-    {'EXIT',{badarg,_}} = (catch M0#{1.0:=v,1.0=>v2}),
-    {'EXIT',{badarg,_}} = (catch M0#{42.0:=v,42:=v2}),
-    {'EXIT',{badarg,_}} = (catch M0#{42=>v1,42.0:=v2,42:=v3}),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},_}} =
+			  (catch T#{nonexisting=>val})
+	      end),
+    Empty = id(#{}),
+    {'EXIT',{{badkey,nonexisting},_}} = (catch Empty#{nonexisting:=val}),
+    {'EXIT',{{badkey,nonexisting},_}} = (catch M0#{nonexisting:=val}),
+    {'EXIT',{{badkey,1.0},_}} = (catch M0#{1.0:=v,1.0=>v2}),
+    {'EXIT',{{badkey,42},_}} = (catch M0#{42.0:=v,42:=v2}),
+    {'EXIT',{{badkey,42.0},_}} = (catch M0#{42=>v1,42.0:=v2,42:=v3}),
 
     ok.
 
@@ -1071,10 +1080,10 @@ t_update_exact_large(Config) when is_list(Config) ->
     M2 = M0#{13.0=>wrong,13.0:=new},
 
     %% Errors cases.
-    {'EXIT',{badarg,_}} = (catch M0#{nonexisting:=val}),
-    {'EXIT',{badarg,_}} = (catch M0#{1.0:=v,1.0=>v2}),
-    {'EXIT',{badarg,_}} = (catch M0#{42.0:=v,42:=v2}),
-    {'EXIT',{badarg,_}} = (catch M0#{42=>v1,42.0:=v2,42:=v3}),
+    {'EXIT',{{badkey,nonexisting},_}} = (catch M0#{nonexisting:=val}),
+    {'EXIT',{{badkey,1.0},_}} = (catch M0#{1.0:=v,1.0=>v2}),
+    {'EXIT',{{badkey,42},_}} = (catch M0#{42.0:=v,42:=v2}),
+    {'EXIT',{{badkey,42.0},_}} = (catch M0#{42=>v1,42.0:=v2,42:=v3}),
 
     ok.
 
@@ -1766,12 +1775,17 @@ t_bif_map_get(Config) when is_list(Config) ->
     "tuple hi" = maps:get({1,1.0}, M1),
     "v3" = maps:get(<<"k2">>, M1),
 
-    %% error case
-    {'EXIT',{badarg, [{maps,get,_,_}|_]}} = (catch maps:get(a,[])),
-    {'EXIT',{badarg, [{maps,get,_,_}|_]}} = (catch maps:get(a,<<>>)),
-    {'EXIT',{bad_key,[{maps,get,_,_}|_]}} = (catch maps:get({1,1}, #{{1,1.0} => "tuple"})),
-    {'EXIT',{bad_key,[{maps,get,_,_}|_]}} = (catch maps:get(a,#{})),
-    {'EXIT',{bad_key,[{maps,get,_,_}|_]}} = (catch maps:get(a,#{b=>1, c=>2})),
+    %% error cases
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,get,_,_}|_]}} =
+			  (catch maps:get(a, T))
+	      end),
+
+    {'EXIT',{{badkey,{1,1}},[{maps,get,_,_}|_]}} =
+	(catch maps:get({1,1}, #{{1,1.0} => "tuple"})),
+    {'EXIT',{{badkey,a},[{maps,get,_,_}|_]}} = (catch maps:get(a, #{})),
+    {'EXIT',{{badkey,a},[{maps,get,_,_}|_]}} =
+	(catch maps:get(a, #{b=>1, c=>2})),
     ok.
 
 t_bif_map_find(Config) when is_list(Config) ->
@@ -1804,8 +1818,10 @@ t_bif_map_find(Config) when is_list(Config) ->
     error = maps:find(1, #{ 1.0  => "float"}),
     error = maps:find({1.0,1}, #{ a=>a, {1,1.0} => "tuple hi"}), % reverse types in tuple key
 
-    {'EXIT',{badarg,[{maps,find,_,_}|_]}} = (catch maps:find(a,id([]))),
-    {'EXIT',{badarg,[{maps,find,_,_}|_]}} = (catch maps:find(a,id(<<>>))),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,find,_,_}|_]}} =
+			  (catch maps:find(a, T))
+	      end),
     ok.
 
 
@@ -1830,8 +1846,10 @@ t_bif_map_is_key(Config) when is_list(Config) ->
     false = maps:is_key(1.0, maps:put(1, "number", M1)),
 
     %% error case
-    {'EXIT',{badarg,[{maps,is_key,_,_}|_]}} = (catch maps:is_key(a,id([]))),
-    {'EXIT',{badarg,[{maps,is_key,_,_}|_]}} = (catch maps:is_key(a,id(<<>>))),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,is_key,_,_}|_]}} =
+			  (catch maps:is_key(a, T))
+	      end),
     ok.
 
 t_bif_map_keys(Config) when is_list(Config) ->
@@ -1845,11 +1863,10 @@ t_bif_map_keys(Config) when is_list(Config) ->
     [4,int,"hi",<<"key">>] = lists:sort(maps:keys(M1)),
 
     %% error case
-    {'EXIT',{badarg,[{maps,keys,_,_}|_]}} = (catch maps:keys(1 bsl 65 + 3)),
-    {'EXIT',{badarg,[{maps,keys,_,_}|_]}} = (catch maps:keys(154)),
-    {'EXIT',{badarg,[{maps,keys,_,_}|_]}} = (catch maps:keys(atom)),
-    {'EXIT',{badarg,[{maps,keys,_,_}|_]}} = (catch maps:keys([])),
-    {'EXIT',{badarg,[{maps,keys,_,_}|_]}} = (catch maps:keys(<<>>)),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,keys,_,_}|_]}} =
+			  (catch maps:keys(T))
+	      end),
     ok.
 
 t_bif_map_new(Config) when is_list(Config) ->
@@ -1921,9 +1938,14 @@ t_bif_map_merge(Config) when is_list(Config) ->
     ok = check_keys_exist(Ks1 ++ Ks2, M11),
 
     %% error case
-    {'EXIT',{badarg,[{maps,merge,_,_}|_]}} = (catch maps:merge((1 bsl 65 + 3), <<>>)),
-    {'EXIT',{badarg,[{maps,merge,_,_}|_]}} = (catch maps:merge(<<>>, id(#{ a => 1}))),
-    {'EXIT',{badarg,[{maps,merge,_,_}|_]}} = (catch maps:merge(id(#{ a => 2}), <<>> )),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,merge,_,_}|_]}} =
+			  (catch maps:merge(#{}, T)),
+		      {'EXIT',{{badmap,T},[{maps,merge,_,_}|_]}} =
+			  (catch maps:merge(T, #{})),
+		      {'EXIT',{{badmap,T},[{maps,merge,_,_}|_]}} =
+			  (catch maps:merge(T, T))
+	      end),
     ok.
 
 
@@ -1962,11 +1984,10 @@ t_bif_map_put(Config) when is_list(Config) ->
     true = is_members([number,wat,3,"hello",<<"other value">>],maps:values(M6)),
 
     %% error case
-    {'EXIT',{badarg,[{maps,put,_,_}|_]}} = (catch maps:put(1,a,1 bsl 65 + 3)),
-    {'EXIT',{badarg,[{maps,put,_,_}|_]}} = (catch maps:put(1,a,154)),
-    {'EXIT',{badarg,[{maps,put,_,_}|_]}} = (catch maps:put(1,a,atom)),
-    {'EXIT',{badarg,[{maps,put,_,_}|_]}} = (catch maps:put(1,a,[])),
-    {'EXIT',{badarg,[{maps,put,_,_}|_]}} = (catch maps:put(1,a,<<>>)),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,put,_,_}|_]}} =
+			  (catch maps:put(1, a, T))
+	      end),
     ok.
 
 is_members(Ks,Ls) when length(Ks) =/= length(Ls) -> false;
@@ -2009,11 +2030,10 @@ t_bif_map_remove(Config) when is_list(Config) ->
     #{ "hi" := "hello", int := 3, 4 := number} = maps:remove(18446744073709551629,maps:remove(<<"key">>,M0)),
 
     %% error case
-    {'EXIT',{badarg,[{maps,remove,_,_}|_]}} = (catch maps:remove(a,1 bsl 65 + 3)),
-    {'EXIT',{badarg,[{maps,remove,_,_}|_]}} = (catch maps:remove(1,154)),
-    {'EXIT',{badarg,[{maps,remove,_,_}|_]}} = (catch maps:remove(a,atom)),
-    {'EXIT',{badarg,[{maps,remove,_,_}|_]}} = (catch maps:remove(1,[])),
-    {'EXIT',{badarg,[{maps,remove,_,_}|_]}} = (catch maps:remove(a,<<>>)),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,remove,_,_}|_]}} =
+	(catch maps:remove(a, T))
+	      end),
      ok.
 
 t_bif_map_update(Config) when is_list(Config) ->
@@ -2036,10 +2056,10 @@ t_bif_map_update(Config) when is_list(Config) ->
 	4 := number, 18446744073709551629 := wazzup} = maps:update(18446744073709551629, wazzup, M0),
 
     %% error case
-    {'EXIT',{badarg,[{maps,update,_,_}|_]}} = (catch maps:update(1,none,{})),
-    {'EXIT',{badarg,[{maps,update,_,_}|_]}} = (catch maps:update(1,none,<<"value">>)),
-    {'EXIT',{badarg,[{maps,update,_,_}|_]}} = (catch maps:update(5,none,M0)),
-
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,update,_,_}|_]}} =
+			  (catch maps:update(1, none, T))
+	      end),
     ok.
 
 
@@ -2066,10 +2086,10 @@ t_bif_map_values(Config) when is_list(Config) ->
     true = is_members([number,3,"hello2",<<"value2">>]++Vs,maps:values(M5)),
 
     %% error case
-    {'EXIT',{badarg,[{maps,values,_,_}|_]}} = (catch maps:values(1 bsl 65 + 3)),
-    {'EXIT',{badarg,[{maps,values,_,_}|_]}} = (catch maps:values(atom)),
-    {'EXIT',{badarg,[{maps,values,_,_}|_]}} = (catch maps:values([])),
-    {'EXIT',{badarg,[{maps,values,_,_}|_]}} = (catch maps:values(<<>>)),
+    do_badmap(fun(T) ->
+		      {'EXIT',{{badmap,T},[{maps,values,_,_}|_]}} =
+			  (catch maps:values(T))
+	      end),
     ok.
 
 t_erlang_hash(Config) when is_list(Config) ->
@@ -2268,8 +2288,10 @@ t_bif_map_to_list(Config) when is_list(Config) ->
 				  <<"hi">>=>v6,3=>v7,"hi"=>v8,hi=>v9,{hi,3}=>v10})),
 
     %% error cases
-    {'EXIT', {badarg,_}} = (catch maps:to_list(id(a))),
-    {'EXIT', {badarg,_}} = (catch maps:to_list(id(42))),
+    do_badmap(fun(T) ->
+		      {'EXIT', {{badmap,T},_}} =
+			  (catch maps:to_list(T))
+	      end),
     ok.
 
 
@@ -2811,6 +2833,26 @@ y_regs_update(Map0, Val0) ->
     _ = id({K1,K2,Val0,Val1}),			%Force use of Y registers.
     Map2.
 
+do_badmap(Test) ->
+    Terms = [Test,fun erlang:abs/1,make_ref(),self(),0.0/id(-1),
+	     <<0:1024>>,<<1:1>>,<<>>,<<1,2,3>>,
+	     [],{a,b,c},[a,b],atom,10.0,42,(1 bsl 65) + 3],
+    [Test(T) || T <- Terms].
+
+%% Test that a module compiled with the OTP 17 compiler will
+%% generate the correct 'badmap' exception.
+badmap_17(Config) ->
+    case ?MODULE of
+	map_SUITE -> do_badmap_17(Config);
+	_ -> {skip,"Run in map_SUITE"}
+    end.
+
+do_badmap_17(Config) ->
+    Mod = badmap_17,
+    DataDir = test_server:lookup_config(data_dir, Config),
+    Beam = filename:join(DataDir, Mod),
+    {module,Mod} = code:load_abs(Beam),
+    do_badmap(fun Mod:update/1).
 
 %% Use this function to avoid compile-time evaluation of an expression.
 id(I) -> I.
diff --git a/erts/emulator/test/map_SUITE_data/badmap_17.beam b/erts/emulator/test/map_SUITE_data/badmap_17.beam
new file mode 100644
index 0000000000..277fc34b94
Binary files /dev/null and b/erts/emulator/test/map_SUITE_data/badmap_17.beam differ
diff --git a/erts/emulator/test/map_SUITE_data/badmap_17.erl b/erts/emulator/test/map_SUITE_data/badmap_17.erl
new file mode 100644
index 0000000000..0ec65e0e33
--- /dev/null
+++ b/erts/emulator/test/map_SUITE_data/badmap_17.erl
@@ -0,0 +1,26 @@
+-module(badmap_17).
+-export([update/1]).
+
+%% Compile this source file with OTP 17.
+
+update(Map) ->
+    try
+	update_1(Map),
+	error(update_did_not_fail)
+    catch
+	error:{badmap,Map} ->
+	    ok
+    end,
+    try
+	update_2(Map),
+	error(update_did_not_fail)
+    catch
+	error:{badmap,Map} ->
+	    ok
+    end.
+
+update_1(M) ->
+    M#{a=>42}.
+
+update_2(M) ->
+    M#{a:=42}.
-- 
cgit v1.2.3