Change the subtag used for maps from 0xB to 0xF

[kostis]

The subtag chosen for maps breaks crucial assumptions in other parts of
the system, in particular the native code generation scheme used in the
HiPE compiler. Therefore it is better to use 'the other' unused subtag.

The main change here is the use of 0xF subtag for maps instead of 0xB.
The rest of the differences are changes to and additions of comments.

One more comment from my part.
I noticed that the file contains the following two lines:

#define FLOAT_SUBTAG            (0x6 << _TAG_PRIMARY_SIZE) /* FLOAT _/
#define EXPORT_SUBTAG           (0x7 << TAG_PRIMARY_SIZE) / FLOAT */

The comment of the second line is most certainly a copy and paste error
and should be appropriately fixed by the OTP developers. More importantly,
it would be great if that subtag (0x7) turned out to be unused and could
be used for maps instead of the 0xF one. This might turn out very handy
in the future. I can elaborate on this, if needed.

[psyeugenic]

Reasonable.

When you say "other parts of the system", what do you mean besides HiPE?

For OTP 18, I will probably need a couple of additional tags. Also, I've considered changing the current tagscheme to allow payload in header where arity is defined by the tag. Does HiPE make a lot of assumption with the current scheme?

[kostis]

Git automatically deleted two lines from my initial commit message because they were beginning with a `#' sign. I've put them back again in the comment of the pull request.

Perhaps the expression "other parts of the system" is a bit exaggerated. The problem I run into is specific to the use of the BINARY_XXX_MASK definition. A grep in the code revealed the following about its use:

$ grep -r BINARY_XXX_MASK .
./lib/hipe/rtl/hipe_tagscheme.erl:-define(BINARY_XXX_MASK,     (16#3 bsl ?TAG_PRIMARY_SIZE)).
./lib/hipe/rtl/hipe_tagscheme.erl:  Mask = ?TAG_HEADER_MASK - ?BINARY_XXX_MASK,
./lib/hipe/rtl/hipe_tagscheme.erl:  Mask = ?TAG_HEADER_MASK - ?BINARY_XXX_MASK,
./erts/emulator/beam/erl_term.h:#define _BINARY_XXX_MASK        (0x3 ... _TAG_PRIMARY_SIZE)
./erts/emulator/beam/erl_term.h:  (((x) & (_TAG_HEADER_MASK-_BINARY_XXX_MASK)) == _TAG_HEADER_EXTERNAL_PID)

The first three lines concern HiPE. The fourth is the definition of the macro and the last one is the definition of is_external_header. This macro is used in:

$ grep -r is_external_header .
./erts/emulator/beam/erl_db_util.c:         ASSERT(is_external_header(u.hdr->thing_word));
./erts/emulator/beam/erl_gc.c:          ASSERT(is_external_header(ptr->thing_word));
./erts/emulator/beam/erl_gc.c:             is_external_header(ptr->thing_word));
./erts/emulator/beam/erl_node_tables.c:     ASSERT(is_external_header(u.hdr->thing_word));
./erts/emulator/beam/erl_message.c:         ASSERT(is_external_header(u.hdr->thing_word));
./erts/emulator/beam/erl_term.h:#define is_external_header(x) \
./erts/emulator/beam/erl_term.h:#define is_external(x) (is_boxed((x)) && is_external_header(*boxed_val((x))))
./erts/emulator/beam/erl_term.h:  (_unchecked_is_boxed((x)) && is_external_header(*_unchecked_boxed_val((x))))

All these places need to be reviewed (and possibly revisited) if 0xB is to be used as a subtag.

[OTP-Maintainer]

Patch has passed first testings and has been assigned to be reviewed

[kostis]

Can I please ask that this is included in master today so that I can also submit a pull request for 'is_map/1' and 'map_size/1' for HiPE?

[sverker]

Accepted and merged to master with an additional commit to fix macro is_external_header

[kostis]

I just noticed that a minor cleanup which was suggested in the comments of my commit message, namely to do something about the (erroneous) comment in the second line below

#define FLOAT_SUBTAG (0x6 << _TAG_PRIMARY_SIZE) /* FLOAT _/
#define EXPORT_SUBTAG (0x7 << TAG_PRIMARY_SIZE) / FLOAT */

was not addressed. Not a big deal, but future generations may appreciate a better comment there.

[nox]

It would be cool if next time you could also amend the commit message itself instead of just the pull request description.