Bits helper

[holub]

Current helper for bit subset looks like BIT(x, bitpos_lo, len) which is similar to substr BIT(x, 3, 5).
For bitwise the such is a bit counterintuitive as position numbered from right to left.

Would it be more user-friendly to have helper similar to:
BIT(x, pos_hi, len) - BIT(x, 7, 5)
BIT(x, pos_hi, pos_lo) - BIT(x, 7, 3), similar to x:7:3

[cuavas]

It’s similar to the C form (x >> lsb) & ((1 << width) - 1) (in your example that would be (x >> 3) & ((1 << 5) - 1)) but less error prone and easier to read.

It’s also directly equivalent to the AArch64 UBFX (Unsigned Bitfield Extract) synthetic. The syntax is:
ubfx <dest>, <src>, #<lsb>, #<width>

With your example, that would be, e.g. ubfx.w r0, r0, #3, #5 – note that it uses the same position and width arguments in the same order as MAME’s BIT helper. Given AArch64 is an instruction set designed with the benefit of a lot of hindsight, I’m going to assume they weren’t completely stupid when they decided to go with this syntax.

If you were to specify the field in terms of most significant and least significant positions, you have more potential pitfalls. What happens if the least significant position is greater than the most significant position, for example BIT(x, 5, 7)? You can’t encounter this situation with the current approach of using unsigned position and width parameters.

It’s not going to change now, anyway.

[rb6502]

Also nobody uses the 3-operand form of BIT because it should be called BITS. Or BITFIELD or EXTRACT or whatever.

[holub]

I do use it but it always drives me to the confusion.
As for me shift + mask is more readable for binary operations than 3 arguments version of the BIT

[cuavas]

I think it’s more popular with people who’ve started dabbling with MAME development more recently. For example cam900 uses the three-parameter form of BIT quite frequently, and there’s substantial use of it in the rewritten Taito F3 video code.

[rb6502]

It being popular with non-native English speakers doesn't really deflect my complaint that it's named incorrectly.

[happppp]

Yes IMO a 3-param BIT function like BIT(x, msb, lsb) is more readable than BIT(x, lsb, count). Though personally I would avoid either of them.

BIT(x, 15, 13) instead of BIT(x, 13, 3)
BIT(x, 7, 0) instead of BIT(x, 0, 8) (but please, just use x & 0xff...)
etc.

Or to put it in words: "I want to extract bit sequence msb to lsb" instead of "I want to extract number of bits starting from lsb"
If you want an equivalent synthetic like your UBFX example:
In Verilog it's x[msb:lsb]
VHDL: x(msb downto lsb)

As for the function body in src/lib/util/coretmpl.h:
old:
return (x >> n) & make_bitmask(w);
new:
return (x >> w) & make_bitmask(1 + n - w);

Why worry about 'pitfalls' when a dev swaps the params around? It simply won't work, just like right now if someone accidentally does BIT(x, count, lsb)

[galibert]

Ewww, I'd get it wrong all the time to have msb inclusive when slices everywhere are exclusive of the end.

…

On Thu, Jun 13, 2024 at 11:08 AM hap ***@***.***> wrote: Yes IMO a 3-param BIT function like BIT(x, msb, lsb) is more readable than BIT(x, lsb, count). Though personally I would avoid either of them. BIT(x, 15, 13) instead of BIT(x, 13, 3) BIT(x, 7, 0) instead of BIT(x, 0, 8) (but please, just use x & 0xff...) etc. As for the function body in src/lib/util/coretmpl.h: old: return (x >> n) & make_bitmask(w); new: return (x >> w) & make_bitmask(1 + n - w); Why worry about 'pitfalls' when a dev swaps the params around? It simply won't work, just like right now if someone accidentally does BIT(x, count, lsb) — Reply to this email directly, view it on GitHub <#114 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACGSF4K2Q5QZFIB7OY4OMETZHFOSFAVCNFSM6AAAAABI4W5US6VHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TONRRGAZTQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[happppp]

To each their own, but I doubt you mean it?
So, if a CPU manual says: "D0-D3 are for ...", you think it's D0,D1,D2? Or if it says "Bits 7..4 of this register ...", it's bit 4,5,6?

[rb6502]

I agree with hap on the basis that datasheets are always in terms of "you want bits 15-11 for the destination register" or whatever. Also, that's consistent with e.g. the PowerPC RLWI* opcodes which are inclusive on the start and end bits specified. But regardless of what the parameters say the multi-bit version should not be called BIT because it does not return a bit.

[happppp]

Then change the last param from numbits to mask, and rename the function to shift_and_mask(x, shift, mask) and realize the uselessness of this method where one would normally just write x >> shift & mask ;)

[rb6502]

I'd rather have EXTRACT(data, 15, 11) for my example, because not having to manually compute a mask is the point of these helpers. To me that's the most intuitive syntax (leftmost bit position is on the left), and it avoids manually computing the mask, which seems to be the point of these helpers.

[happppp]

Sure I understand, but lowercase and more descriptive please, such as extract_bits(data, 15, 11)

[rb6502]

It's a preprocessor macro, so by MAME style it should be all uppercase, same as LOGMASKED() and things like that.

[happppp]

Not exactly, see https://github.com/mamedev/mame/blob/master/src/lib/util/coretmpl.h#L607
(in the past, MAME's BIT and BITSWAP(now lowercase) used to be macros)

[rb6502]

Oh, yeah, everything else in that file is lowercase. Good call.

[happppp]

I just tried renaming the 3-param BIT function to extract_bits, but the compiler complains and suggests util::extract_bits. 1: why? 2: nobody would want to type util::extract_bits(data, pos, num_bits)

edit: ah, I was missing using util::extract_bits; in emucore.h

I might also suggest:

template <typename T, typename U, typename V, typename W> constexpr T insert_bits(T x, U y, V n, W w)
{
	return (make_bitmask<T>(8 * sizeof(x)) ^ make_bitmask<T>(w) << n) | (y & make_bitmask<T>(w)) << n;
}

And there could also be set_bit(x, y) (alias for insert_bits(x, 1, y, 1)) and clear_bit(x, y) (alias for insert_bits(x, 0, y, 1))

[ajrhacker]

The problem with the proposed BIT(x, pos_hi, pos_lo) syntax is that it requires two values to be calculated rather than just one in the fairly common case where a fixed-width field needs to be extracted from one of several positions.

The PDP-10 instructions LDB and DPB provide further historic precedent for designating the position of a bitfield by the number of its lowest bit and its size in bits. This case was slightly complicated because the assembler syntax for byte pointers upheld the old big-endian convention of numbering the bits from left to right, and but they were actually encoded the opposite way (0 = LSB and 35 = MSB).