Pass correct live value in BIFs

[jgonet]

If BIF did GC and tried to preserve input registers and args with memory_ensure_free_with_roots(ctx, TERMS_N, live, ctx->x, MEMORY_CAN_SHRINK) it would invalidate registers anyways because live=0.

Now, GC BIFs can keep them at cost of temporary memory increase until next GC is done.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[pguyot]

I'm not sure this is a required change. GC in binary_part was wrongly invoked without arg1 in roots.

[bettio]

I'm not sure this is a required change. GC in binary_part was wrongly invoked without arg1 in roots.

Actually it is not a required change (right now), but it might be a good idea for avoiding any future headache.

When we do GC inside a BIF function called with gc_bif instruction, we must do memory_ensure_free_with_roots(..., live, ctx->x, ...), and I believe BIF functions can safely assume that max-x-reg <= live.
If we always use 0 as live value when calling BIF functions as NIFs we may need to add some code for distinguishing the 2 cases, or something like memory_ensure_free_with_roots(..., MAX(live, used_x_regs), ctx->x, ...) that is not very convenient.

So this change doesn't hurt at all and it allow us to avoid this situation in the future.

Let me know if there is anything wrong with my reasoning.

[jgonet]

I wanted to add that currently this code is called from two code-paths – CALL_LAST (which AFAIK comes from tail calls and is fine in that case) and apply (which I'm worried isn't correct but I can't construct a failing example).

If the original code stays, this warrants at least a comment and maybe even some debug-only asserts to ensure we're not calling it from the wrong context.

[pguyot]

NIF functions don't have live, as the compiler allows them to scratch all x registers. NIF implementations calling memory_ensure_free must put args in roots if args are referred to below.

GC BIF functions do have live, as the compiler requires them to preserve the first live x registers.

If we use the trick as suggested to pass argc as live to GC BIF functions when calling them as NIFs, we assume that when the GC BIF functions are called as BIFs, the compiler sets lives in such a way that the parameters are either within the first live x registers or on the stack. I thought it would not always hold but it seems to be the case with OTP27. Please also note that it seems that GC BIFs no longer GC on OTP, instead they allocate fragments.

[bettio]

Regardless the outcome of this discussion, we still have a number of occurrences (where we call gcbif1_ptr) where we didn't update live parameter accordingly as we did here. (just search for gcbif1_ptr).

[bettio]

If we use the trick as suggested to pass argc as live to GC BIF functions when calling them as NIFs, we assume that when the GC BIF functions are called as BIFs, the compiler sets lives in such a way that the parameters are either within the first live x registers or on the stack.

Lets's think about the whole discussion this way:

1. we must preserve at least live registers when GC BIFs are called using OP_GC_BIF1, ..., OP_GC_BIF3 opcodes, this looks mandatory right now.
2. if we are unsure if live is always >= argc we might add some kind of assert (that is discarded in release builds) in opcodesswitch.h. If we do that, let's keep this changes on a different commit.
3. let's supposed for a moment a hypothetical function that when we call memory_ensure_free we just need to preserve x[0]. In that specific case we might have to distinguish between the NIF (OP_CALL_EXT, etc...) and the BIF (OP_GC_BIF1), because in the latter codepath we still must preserve live registers (because of 1.). So we either check if live == 0 in each specific case, to decide in which codepath we are, or we just blindly always preserve live x-registers (assuming that always live >= argc)
4. if we check this in each specific case, let's leave a note in bif.c about this practice, to avoid any future mistake
5. in release-0.6 we are not yet using fragments as far as i know in our BIFs, so live paramater still matters.

[pguyot]

Right.

To clarify further:

• we don't know we need to preserve x[0] unless we know we are called as a NIF. If gc bif opcodes are used we don't know the origin of the parameters (they can be y).

• we don't use fragments in any gc bifs implementation whatever the branch, unlike BEAM. I mentioned this as a warning that at some point live might no longer be accurate because BEAM doesn't use it since they use fragments. In other words, gc bifs no longer gc in BEAM but they still may gc in AtomVM. A potential solution to this would be to avoid gc in gc bifs (using appropriate option to memory_ensure_free_opt), but this could have a performance penalty.

[jgonet]

@bettio

Regardless the outcome of this discussion, we still have a number of occurrences (where we call gcbif1_ptr) where we didn't update live parameter accordingly as we did here. (just search for gcbif1_ptr).

Can you give locations? I don't see any after searching for gcbif1_ptr .

[bettio]

In opcodesswitch.h:
maybe_call_native: 1615-1623
OP_CALL_EXT: 2072-2075
OP_CALL_EXT_LAST: 2206-2209
OP_CALL_EXT_ONLY: 3688-3691

[jgonet]

Ah, my bad, wrongly interpreted global search in my editor. Thanks!

[jgonet]

@bettio did changes to opcodesswitch from main ever got to release-0.6?

• release 0.6 line 2072
• main line 2072