Hi Jürgen,
This is a follow-up to my earlier big-endian mail (_Atomic_word and CDR).
It covers the third big-endian problem I found on sparc64, packed Boolean
ravels, plus some packed-value bugs in 2.0 that affect all architectures.
THE BYTE ORDER PROBLEM
The packed Boolean ravel is accessed in two different ways:
- as uint64_t words, bit b being bit b&63 of word b>>6;
- as bytes, bit b being bit b&7 of byte b>>3.
These agree only on little-endian machines. On a big-endian host (sparc64,
s390x, ppc64, ...), a value written one way and read the other comes back
scrambled. In 2.0, for example, Q ≡ 40 ⎕CR Q fails in Quad_CR.tc.
In trunk the word-wise side has grown: the ScalarFunction bool fast paths,
the ρ fast path in Bif_F12_RHO.cc, ⍸ in Bif_F12_INTERVAL_INDEX.cc, 41 ⎕CR,
try_implode(), 40 ⎕CR, and all of the Ravel.cc writers. The comment in
Bif_F12_ELEMENT.cc already states that RPT_BOOL "is addressed ... as
uint64_t words". But a few readers still go byte-wise:
- Ravel::bool_fetcher()
- BoolRavel::get_int_value() and get_near_bool()
- the RPT_BOOL case of the explode-to-Cells loop in Value.cc
- the bool -> int64 upgrade loop in Value.cc
- the legacy (flg 0x10) ⁹ loader in Archive.cc, which writes byte-wise
The trunk patch below changes these to word-wise access through a new
Ravel::get_packed_bit(). The legacy loader now puts archive byte n into
bits 8(n&7) of word n>>3. That keeps the archive format unchanged, and it
is the same as before on little-endian.
ADDITIONAL BUGS IN 2.0, ALREADY FIXED IN TRUNK
The attached 0014-packed-boolean-byte-order.patch is what Debian applies to
2.0. Besides the byte order (where it made the writers little-endian
instead, because 2.0 had fewer word-wise users), it fixes some bugs that
trunk has since fixed in its own way:
- 41 ⎕CR and the )SAVE writer located the bits via &get_cfirst(). For a
packed value that is the static boolean cell from packed_fetcher(), not the
ravel. So on every architecture a )SAVEd packed variable came back as
garbage after )LOAD.
- The loader's next_ravel_Byte() always wrote byte 0.
- Bytes < 0x10 were written as a single hex digit.
- The ⁹ on continuation lines (values over 512 bits) ended parsing.
- 41 ⎕CR returned Idx0 instead of its result.
I'm sending it in case it's useful for a 2.x maintenance release. Trunk
does not need it.
One thing you may want to know about: trunk can't )LOAD a packed variable
saved by 2.0. 2.0 writes flg="0x4400" and a bytes="⁹..." attribute, trunk's
legacy path looks for flg 0x10, and the )LOAD fails with DOMAIN ERROR.
Since an unpatched 2.0 saved garbage for these anyway, this probably only
matters for workspaces saved with Debian's patched 2.0.
TESTING
- 2.0 with 0014: built and tested on sparc64 (Debian porter box) and amd64.
All upstream testcases we run pass on both. The one exception is the
Format_by_spec / ZZZ0_Standard_20x long double precision difference on
sparc64, which is unrelated.
- Trunk patch: amd64 only. I have not run it on big-endian hardware.
- Quad_CR.tc and ZZ17 pass.
- The full testcase run gives exactly the same set of failures with and
without the patch. Those failures are all environment-related (files, GTK,
SQL, ...).
- Packed and unpacked results agree for ~, +/, ∧ with ⌽, ρ, and 41/40 ⎕CR.
- A hand-made legacy (flg 0x10, ⁹) archive )LOADs correctly.
I'd be glad to run trunk on sparc64 if that would help.
Both patches are also attached, in case the inline copy gets mangled.
Cheers,
--Barak.
--- a/trunk/src/Archive.cc
+++ b/trunk/src/Archive.cc
@@ -2576,7 +2576,11 @@
// per-element cases above, is not bounded by Z.more()).
//
const ShapeItem max_bytes = ((Z.element_count() + 63) >> 6) << 3;
- uint8_t * const dst = reinterpret_cast<uint8_t *>(&Z.get_wfirst());
+ // byte n of the archive holds items 8n...8n+7, i.e. bits
+ // 8(n&7)... of uint64_t word n>>3 of the ravel.
+ //
+ uint64_t * const dst = reinterpret_cast<uint64_t *>(&Z.get_wfirst());
+ memset(dst, 0, max_bytes);
ShapeItem count = 0;
for (;;)
@@ -2594,7 +2598,11 @@
const int converted = end - cc;
if (converted == 0) break; // nothing converted
- if (count < max_bytes) dst[count++] = byte; // bounded
+ if (count < max_bytes) // bounded
+ {
+ dst[count >> 3] |= uint64_t(byte) << ((count & 7) << 3);
+ ++count;
+ }
if (converted == 1) { ++input; break; }
}
return input;
--- a/trunk/src/Ravel.hh
+++ b/trunk/src/Ravel.hh
@@ -191,6 +191,14 @@
Cell & not_used)
{ return cells[offset]; }
+ /// return bit \b idx of a bool-packed ravel. The ravel is written and
+ /// read (e.g. by the ScalarFunction fast paths) as uint64_t words, bit b
+ /// being bit b&63 of word b>>6. Accessing it bytewise instead (bit b&7 of
+ /// byte b>>3) gives the same result on little-endian machines only.
+ static APL_Integer get_packed_bit(const Cell * cells, ShapeItem idx)
+ { return (reinterpret_cast<const uint64_t *>(cells)[idx >> 6]
+ >> (idx & 63)) & 1; }
+
/// fetch function for packed (bit-array) boolean ravels.
/// @param offset ravel index (0-based)
/// @param cells pointer to the packed bit array cast to Cell *
@@ -198,8 +206,7 @@
static const Cell & bool_fetcher(ShapeItem offset, const Cell * cells,
Cell & cache)
{
- new (&cache) IntCell((1 << (offset & 7) &
- reinterpret_cast<const uint8_t *>(cells)[offset >> 3]) ? 1 : 0);
+ new (&cache) IntCell(get_packed_bit(cells, offset));
return cache;
}
@@ -530,13 +537,13 @@
BoolRavel() : Ravel(upgrade_tag{}) {}
virtual APL_Integer get_int_value(ShapeItem idx) const override
- { return (reinterpret_cast<const uint8_t *>(cells)[idx >> 3] >> (idx &
7)) & 1; }
+ { return get_packed_bit(cells, idx); }
virtual APL_Integer get_near_int(ShapeItem idx) const override
{ return get_int_value(idx); }
virtual bool get_near_bool(ShapeItem idx) const override
- { return (reinterpret_cast<const uint8_t *>(cells)[idx >> 3] >> (idx &
7)) & 1; }
+ { return get_packed_bit(cells, idx); }
virtual APL_Float get_real_value(ShapeItem idx) const override
{ return APL_Float(get_int_value(idx)); }
--- a/trunk/src/Value.cc
+++ b/trunk/src/Value.cc
@@ -1193,10 +1193,7 @@
case RPT_BOOL:
rev_loop(i, N)
{
- const uint8_t & src =
- reinterpret_cast<const int8_t *>(ravel.cells)[i >> 3];
-
- new (--dst) IntCell((src >> (i & 7)) & 1);
+ new (--dst) IntCell(Ravel::get_packed_bit(ravel.cells, i));
}
break;
@@ -1411,8 +1408,7 @@
Assert1(get_ravel_type() == RPT_BOOL);
rev_loop(i, N)
{
- auto src = reinterpret_cast<const int8_t *>(ravel.cells)[i >> 3];
- *--dst = int64_t((src >> (i & 7)) & 1);
+ *--dst = int64_t(Ravel::get_packed_bit(ravel.cells, i));
}
flags.ravel_type = RPT_INT64;
--- a/trunk/src/Archive.cc
+++ b/trunk/src/Archive.cc
@@ -2576,7 +2576,11 @@
// per-element cases above, is not bounded by Z.more()).
//
const ShapeItem max_bytes = ((Z.element_count() + 63) >> 6) <<
3;
- uint8_t * const dst = reinterpret_cast<uint8_t
*>(&Z.get_wfirst());
+ // byte n of the archive holds items 8n...8n+7, i.e. bits
+ // 8(n&7)... of uint64_t word n>>3 of the ravel.
+ //
+ uint64_t * const dst = reinterpret_cast<uint64_t
*>(&Z.get_wfirst());
+ memset(dst, 0, max_bytes);
ShapeItem count = 0;
for (;;)
@@ -2594,7 +2598,11 @@
const int converted = end - cc;
if (converted == 0) break; // nothing converted
- if (count < max_bytes) dst[count++] = byte; // bounded
+ if (count < max_bytes) // bounded
+ {
+ dst[count >> 3] |= uint64_t(byte) << ((count & 7) <<
3);
+ ++count;
+ }
if (converted == 1) { ++input; break; }
}
return input;
--- a/trunk/src/Ravel.hh
+++ b/trunk/src/Ravel.hh
@@ -191,6 +191,14 @@
Cell & not_used)
{ return cells[offset]; }
+ /// return bit \b idx of a bool-packed ravel. The ravel is written and
+ /// read (e.g. by the ScalarFunction fast paths) as uint64_t words, bit b
+ /// being bit b&63 of word b>>6. Accessing it bytewise instead (bit b&7 of
+ /// byte b>>3) gives the same result on little-endian machines only.
+ static APL_Integer get_packed_bit(const Cell * cells, ShapeItem idx)
+ { return (reinterpret_cast<const uint64_t *>(cells)[idx >> 6]
+ >> (idx & 63)) & 1; }
+
/// fetch function for packed (bit-array) boolean ravels.
/// @param offset ravel index (0-based)
/// @param cells pointer to the packed bit array cast to Cell *
@@ -198,8 +206,7 @@
static const Cell & bool_fetcher(ShapeItem offset, const Cell * cells,
Cell & cache)
{
- new (&cache) IntCell((1 << (offset & 7) &
- reinterpret_cast<const uint8_t *>(cells)[offset >> 3]) ? 1 : 0);
+ new (&cache) IntCell(get_packed_bit(cells, offset));
return cache;
}
@@ -530,13 +537,13 @@
BoolRavel() : Ravel(upgrade_tag{}) {}
virtual APL_Integer get_int_value(ShapeItem idx) const override
- { return (reinterpret_cast<const uint8_t *>(cells)[idx >> 3] >> (idx &
7)) & 1; }
+ { return get_packed_bit(cells, idx); }
virtual APL_Integer get_near_int(ShapeItem idx) const override
{ return get_int_value(idx); }
virtual bool get_near_bool(ShapeItem idx) const override
- { return (reinterpret_cast<const uint8_t *>(cells)[idx >> 3] >> (idx &
7)) & 1; }
+ { return get_packed_bit(cells, idx); }
virtual APL_Float get_real_value(ShapeItem idx) const override
{ return APL_Float(get_int_value(idx)); }
--- a/trunk/src/Value.cc
+++ b/trunk/src/Value.cc
@@ -1193,10 +1193,7 @@
case RPT_BOOL:
rev_loop(i, N)
{
- const uint8_t & src =
- reinterpret_cast<const int8_t *>(ravel.cells)[i >> 3];
-
- new (--dst) IntCell((src >> (i & 7)) & 1);
+ new (--dst) IntCell(Ravel::get_packed_bit(ravel.cells, i));
}
break;
@@ -1411,8 +1408,7 @@
Assert1(get_ravel_type() == RPT_BOOL);
rev_loop(i, N)
{
- auto src = reinterpret_cast<const int8_t *>(ravel.cells)[i >> 3];
- *--dst = int64_t((src >> (i & 7)) & 1);
+ *--dst = int64_t(Ravel::get_packed_bit(ravel.cells, i));
}
flags.ravel_type = RPT_INT64;
From: "Barak A. Pearlmutter" <[email protected]>
Date: Sun, 1 Nov 2026 17:00:00 +0000
Subject: packed Boolean ravels: fix byte order, )SAVE/)LOAD, 41 âCR
All readers of packed Boolean ravels (Value::packed_fetcher(),
Value::explode(), the workspace archive, 41 âCR) access them bytewise:
item b is bit b&7 of byte b>>3. The writers (Value::try_implode() and
40 âCR) assembled 64-bit chunks and stored them as host-order uint64_t,
which matches only on little-endian hosts. On big-endian hosts packed
values read back scrambled, e.g. Q â¡ 40 âCR QP fails in Quad_CR.tc.
Store chunks little endian via a new Value::store_packed_chunk().
Also fix two readers that located the packed bits via &get_cfirst():
for a packed value that is a reference to a static boolean Cell returned
by packed_fetcher(), not the ravel, so 41 âCR and )SAVE (XML archive)
read unrelated memory -- on all architectures a saved packed variable
came back as garbage after )LOAD. Add Value::get_packed_bytes() and
use it there. Loading packed values from an XML workspace was broken
too: Value::next_ravel_Byte() always wrote byte 0, single hex digits
were emitted for bytes < 0x10, and the â¹ on continuation lines (values
longer than 512 bits) ended parsing; write each byte at its index via
a new Value::set_packed_byte(), emit two hex digits, and skip â¹.
And make 41 âCR return the unpacked value it builds
rather than Idx0 (an empty vector).
Forwarded: no
---
--- a/src/Quad_CR.cc
+++ b/src/Quad_CR.cc
@@ -1645,13 +1645,13 @@
bit += bit;
if (bit == 0) // uint64_t bit complete
{
- bits[b >> 6] = chunk;
+ Value::store_packed_chunk(bits + (b >> 6), chunk);
chunk = 0;
bit = 1;
}
}
- if (chunk) bits[Z_len - 1] = chunk; // rest bits
+ if (chunk) Value::store_packed_chunk(bits + Z_len - 1, chunk); // rest bits
Value_P Z(B->get_shape(), bits, LOC);
return Z;
@@ -1669,11 +1669,12 @@
Value_P Z(B->get_shape(), LOC);
const ShapeItem B_len = B->element_count();
-const uint8_t * bits = reinterpret_cast<const uint8_t *>(&B->get_cfirst());
+const uint8_t * bits = B->get_packed_bytes();
loop(b, B_len) Z->next_ravel_Int((bits[b >> 3] & 1ULL << (b & 7)) ? 1 : 0);
- return Idx0(LOC);
+ Z->check_value(LOC);
+ return Z;
}
//----------------------------------------------------------------------------
Value_P
--- a/src/Value.cc
+++ b/src/Value.cc
@@ -3108,11 +3108,11 @@
zbit += zbit; // next bit
if (zbit) continue; // more bits in same chunk
- bits[z++] = zchunk;
+ store_packed_chunk(bits + z++, zchunk);
zchunk = 0;
}
- if (zchunk) bits[Z_len - 1] = zchunk; // rest bits
+ if (zchunk) store_packed_chunk(bits + Z_len - 1, zchunk); // rest bits
if (error_reason)
{
--- a/src/Value.hh
+++ b/src/Value.hh
@@ -534,6 +534,21 @@
static const Cell & cell_fetcher(ShapeItem offset, const Cell * ravel)
{ return ravel[offset]; }
+ /// store a 64-bit chunk of a packed ravel at \b dest. Packed ravels
+ /// are accessed bytewise (bit b is bit b&7 of byte b>>3), so the
+ /// chunk is stored little endian regardless of the host byte order.
+ static void store_packed_chunk(uint64_t * dest, uint64_t chunk)
+ {
+ uint8_t * bytes = reinterpret_cast<uint8_t *>(dest);
+ loop(b, 8) { bytes[b] = chunk & 0xFF; chunk >>= 8; }
+ }
+
+ /// return the bytes of a packed ravel. Note that get_cfirst() and
+ /// friends of a packed value return static boolean cells, not a
+ /// pointer into the ravel.
+ const uint8_t * get_packed_bytes() const
+ { Assert(is_packed()); return reinterpret_cast<const uint8_t *>(ravel); }
+
/// return the offset'th ravel cell (of an pack'ed ravel)
static const Cell & packed_fetcher(ShapeItem offset, const Cell * ravel)
{ return 1 << (offset & 7) &
@@ -607,6 +622,14 @@
/// case ravel is a uint8_t * and not a Cell * !!!
inline void next_ravel_Byte(uint8_t byte);
+ /// set byte \b idx of a packed ravel to \b byte
+ void set_packed_byte(ShapeItem idx, uint8_t byte)
+ {
+ Assert(is_packed());
+ Assert(idx < (nz_element_count() + 7) >> 3);
+ reinterpret_cast<uint8_t *>(ravel)[idx] = byte;
+ }
+
/// initialize the next ravel cell with a pointer to another Cell
inline void next_ravel_Lval(Cell * target, Value * target_owner);
--- a/src/Archive.cc
+++ b/src/Archive.cc
@@ -234,7 +234,7 @@
int space = do_indent();
if (v.is_packed()) // Value has a packed (non-Cell) ravel
{
- const uint8_t * bytes = reinterpret_cast<const uint8_t *>(C);
+ const uint8_t * bytes = v.get_packed_bytes();
const ShapeItem byte_count = (len + 7)/8;
// print the start of the XML element
@@ -252,7 +252,7 @@
loop(b, byte_count) // print with max. 64 bytes per line
{
if (b && (b & 0x3F) == 0) outf << "\n " << UNI_PAD_U9;
- outf << (bytes[b] & 0xFF);
+ outf << setw(2) << (bytes[b] & 0xFF);
}
outf << nohex << "\"/>" << endl;
--indent;
@@ -1952,11 +1952,13 @@
case UNI_PAD_U9: // packed boolean, e.g. â¹
Assert(Z.is_packed());
- for (;;)
+ for (ShapeItem byte_idx = 0;;)
{
const uint8_t c0 = input[0];
const uint8_t c1 = input[1];
if (c0 <= ' ') { ++input; continue; } // whitespace
+ if (c0 == 0xE2 && c1 == 0x81 && uint8_t(input[2]) == 0xB9)
+ { input += 3; continue; } // â¹ on continuation line
if (c0 == '"') return input + 1; // end of ravel
const char cc[3] = { char(c0), char(c1), 0 };
@@ -1966,7 +1968,7 @@
const int converted = end - cc;
if (converted == 0) return input; // nothing converted
- Z.next_ravel_Byte(byte); // > 0 bytes converted
+ Z.set_packed_byte(byte_idx++, byte); // > 0 bytes converted
if (converted == 1) return input + 1;
}
