erts: add erlang:ctz/1 and erlang:popcount/1 BIFs

[saleyn]

Adds functionality and tests for efficient bit operations on 64-bit values that implement compiler built-in functions.

• Implement ctz/1, popcount/1 using compiler builtins with portable fallbacks
• Export and document BIF stubs in erlang.erl
• Implement JIT optimized versions of these functions
• Add Common Test suite bitops_SUITE.erl
• Update bif.tab entries for the new BIFs
• Ensure build links the new object: add erl_bif_bitops.o to Makefile.in and regenerate build files
• Recompile the preloaded erlang.beam

[github-actions]

CT Test Results

    3 files    136 suites   56m 16s ⏱️
1 682 tests 1 625 ✅ 56 💤 1 ❌
2 325 runs  2 250 ✅ 74 💤 1 ❌

For more details on these failures, see this check.

Results for commit 7b4efd8.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[jhogberg]

Thanks for the PR! We're going to review this once most of us are back from summer vacation. One thing that I'm sure will be brought up is that this is limited to 64 bits, whereas Erlang uses arbitrary precision arithmetic. A fast implementation would be relatively trivial as all three operations can be done in a data-parallel manner.

[saleyn]

@jhogberg thanks for the update. One CI action I cannot get to pass is this, which requires maintainers to modify the preloaded erlang.beam:

Run PERMISSION=$(gh api \
Error: Workflow failed: Only maintainers can make modifications to '*.beam' files:
erts/preloaded/ebin/erlang.beam
Error: Process completed with exit code 1.

I added support for bignums in the commit below.

[jhogberg]

@jhogberg thanks for the update. One CI action I cannot get to pass is this, which requires maintainers to modify the preloaded erlang.beam:

Yes, we'll need to do that for you later on.

I added support for bignums in the commit below.

I don't like that it still assumes 64 bits, highest_set_bit and lowest_set_bit (or whatever to name them) that returns the respective position from 0 leaves that to the user.

I also think the implementation is a bit too ambitious, I'd prefer just a dead-simple C implementation that ignores SIMD/__builtin_xyz because in practice the BIF will be a fallback only for use in apply/3 with the JIT doing all the heavy lifting. Implementing them in the JIT is pretty quick once the semantics have been decided. :)

[saleyn]

@jhogberg thanks for the update. One CI action I cannot get to pass is this, which requires maintainers to modify the preloaded erlang.beam:

Yes, we'll need to do that for you later on.

I added support for bignums in the commit below.

I don't like that it still assumes 64 bits, highest_set_bit and lowest_set_bit (or whatever to name them) that returns the respective position from 0 leaves that to the user.

In the last commit, there's no more 64-bit implicit limitation, it just handles 64bit integers differently (built-in GCC) from bignums.

I also think the implementation is a bit too ambitious, I'd prefer just a dead-simple C implementation that ignores SIMD/__builtin_xyz because in practice the BIF will be a fallback only for use in apply/3 with the JIT doing all the heavy lifting. Implementing them in the JIT is pretty quick once the semantics have been decided. :)

You are right, I didn't think about JIT. I will remove SIMD optimization, leaving only bignum support from the last commit. Or should I leave it for reference (I assume in JIT the same implementation would be needed), and remove once implemented in JIT?

[jhogberg]

In the last commit, there's no more 64-bit implicit limitation, it just handles 64bit integers differently (built-in GCC) from bignums.

It assumes a bit width of 64, which doesn't really make sense for bignums (cf. ?assertEqual(0, erlang:clz(BigNum100))). It would be better to have a "get position of highest set bit" operation that's independent of bit width.

You are right, I didn't think about JIT. I will remove SIMD optimization, leaving only bignum support from the last commit. Or should I leave it for reference (I assume in JIT the same implementation would be needed), and remove once implemented in JIT?

You can probably do that part yourself. Look at how erlang:yield/0 is turned into an i_yield instruction in ops.tab, which is implemented in instr_common.cpp. Then look at how i_unary_minus is implemented, specifically the "body" variant of it, and how its bignum fallback is implemented in a shared code fragment to reduce code size (you can declare these in beam_asm_global.hpp.pl).

(You can ignore the emit_enter_runtime stuff since there's no need to call the C implementation.)

[saleyn]

It assumes a bit width of 64, which doesn't really make sense for bignums (cf. ?assertEqual(0, erlang:clz(BigNum100))). It would be better to have a "get position of highest set bit" operation that's independent of bit width.

I see what you meant, and I agree, in presence of bignums the clz/1 makes no sense, so I deleted it completely.

Look at how erlang:yield/0 is turned into an i_yield instruction in ops.tab, which is implemented in instr_common.cpp. Then look at how i_unary_minus is implemented, specifically the "body" variant of it, and how its bignum fallback is implemented in a shared code fragment to reduce code size (you can declare these in beam_asm_global.hpp.pl).

I followed your hints, and added JIT implementation accompanied by test cases.

[saleyn]

@jhogberg, please review my latest changes, and if you could, please check for the two failing CI checks. I am not sure if the emulator failing test is related to my changes.

[jhogberg]

I see what you meant, and I agree, in presence of bignums the clz/1 makes no sense, so I deleted it completely.

"Get position of highest (set) bit" makes sense however, and can be used to implement clz/1 on any bit-width.

I followed your hints, and added JIT implementation accompanied by test cases.

That implementation won't run, they're normal BIFs in implementation and bif.tab, but the pattern matching in ops.tab has them as GC BIFs (which would also require additional compiler support). My instructions were probably a bit to unclear.

Thanks for giving it a shot though, if we accept these, we'll take care of it. :-)

@jhogberg, please review my latest changes, and if you could, please check for the two failing CI checks. I am not sure if the emulator failing test is related to my changes.

It looks unrelated to me too.

[saleyn]

I see what you meant, and I agree, in presence of bignums the clz/1 makes no sense, so I deleted it completely.

"Get position of highest (set) bit" makes sense however, and can be used to implement clz/1 on any bit-width.

I followed your hints, and added JIT implementation accompanied by test cases.

That implementation won't run, they're normal BIFs in implementation and bif.tab, but the pattern matching in ops.tab has them as GC BIFs (which would also require additional compiler support). My instructions were probably a bit to unclear.

Thanks for giving it a shot though, if we accept these, we'll take care of it. :-)

@jhogberg, please review my latest changes, and if you could, please check for the two failing CI checks. I am not sure if the emulator failing test is related to my changes.

It looks unrelated to me too.

Ok, so I'll leave it up to you to fill in the gaps if the team decides to merge.