Skip to content

Commit da4ffbd

Browse files
committed
Say "shuffle sequence" rather than "ladder" in the transpose comments
Comment-only: both headers are byte-identical once comments are stripped, and clang-format is no worse than HEAD on each. An earlier pass dropped "ladder" from the commit messages as a coined word for what is just a sequence of shuffles, but left it in the code comments, so the two disagreed. All eight occurrences now read "shuffle sequence", "shuffles", or in the tabulated instruction count for an 8x8 block, "sequence" -- the (24 shuffles) column keeps its position. The 512-bit note is the one that changes meaning slightly: "needs vpermi2d ladders" becomes "needs vpermi2d shuffles", which is what vpermi2d is.
1 parent 26b5a09 commit da4ffbd

2 files changed

Lines changed: 10 additions & 9 deletions

File tree

‎cpp/src/arrow/util/fastlanes/interleaved_pfor.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,12 +155,12 @@ inline size_t InterleavedPforMaxEncodedSize(size_t n) {
155155
//
156156
// for each 8-lane slice lb, for each 8-row block rb:
157157
// eight registers <- unpack rows rb..rb+7, lanes lb..lb+7 (8-16 loads)
158-
// the unpacklo/unpackhi/permute2x128 ladder (24 shuffles)
158+
// the unpacklo/unpackhi/permute2x128 sequence (24 shuffles)
159159
// eight 32-byte stores across out[lb..lb+7][rb..rb+7] (8 stores)
160160
//
161161
// Load and store counts per block are then identical to the file-order kernel's
162162
// (each packed word read once, each output value written once); the only thing
163-
// FL_ORDER pays over file order is the shuffle ladder. lb is the outer loop on
163+
// FL_ORDER pays over file order is the shuffles. lb is the outer loop on
164164
// purpose: a fixed lb writes out[lb * 32, lb * 32 + 256), one contiguous 1 KiB
165165
// run, whereas rb outer would stride 128 bytes across the whole 4 KiB block.
166166
// ---------------------------------------------------------------------------
@@ -172,8 +172,8 @@ namespace internal {
172172
// needs it too, and that header is upstream of this one.
173173

174174
// One 8x8 tile: unpack eight rows' worth of an 8-lane slice, transpose in
175-
// registers, store across eight output rows. The ladder is the same one
176-
// Transpose32x32Avx2 uses; only the source of r0..r7 differs.
175+
// registers, store across eight output rows. The shuffle sequence is the same
176+
// one Transpose32x32Avx2 uses; only the source of r0..r7 differs.
177177
template <uint32_t w, bool kHasBias, uint32_t rb>
178178
ARROW_FORCE_INLINE void FlUnpackTile(const uint32_t* ARROW_RESTRICT packed,
179179
int32_t* ARROW_RESTRICT out, size_t lb,
@@ -255,7 +255,8 @@ namespace internal {
255255

256256
// One 4x4 tile, the NEON counterpart of FlUnpackTile: unpack four rows' worth of
257257
// a 4-lane slice, transpose in registers, store across four output rows. The
258-
// ladder is the one Transpose32x32Neon uses; only the source of r0..r3 differs.
258+
// shuffle sequence is the one Transpose32x32Neon uses; only the source of
259+
// r0..r3 differs.
259260
// Four-wide rather than eight-wide because that is the register width, which
260261
// costs four times as many tiles per block but the same number of loads and
261262
// stores -- a 16-byte store of four output values either way.

‎cpp/src/arrow/util/fastlanes/transposed_delta.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,13 +445,13 @@ inline void Transpose32x32Neon(const uint32_t* ARROW_RESTRICT grid,
445445

446446
#ifdef ARROW_TRANSPOSED_DELTA_AVX2
447447
// Sixteen 8x8 transposes, each done entirely in registers: eight 32-byte loads
448-
// down a column of the grid, an unpack ladder, eight 32-byte stores across a row
448+
// down a column of the grid, a shuffle sequence, eight 32-byte stores across a row
449449
// of the output. A permutation is the one thing in this file the autovectorizer
450450
// cannot do -- it is not a lane-wise operation, and GCC lowers the scalar
451451
// version below to 1024 four-byte loads at a 128-byte stride plus 1024 scalar
452452
// stores. This issues 128 loads and 128 stores instead, so the block costs
453453
// shuffle throughput rather than memory-op throughput. 256-bit and not 512-bit
454-
// on purpose: a 16x16 register block needs vpermi2d ladders to cross the extra
454+
// on purpose: a 16x16 register block needs vpermi2d shuffles to cross the extra
455455
// lane boundary, and on this microarchitecture 512-bit buys nothing for the
456456
// unpack either.
457457
inline void Transpose32x32Avx2(const uint32_t* ARROW_RESTRICT grid,
@@ -570,7 +570,7 @@ inline void PrefixSumAndTransposeNeon(const uint32_t* ARROW_RESTRICT grid,
570570
// The loop nest is the transpose's, with the blocking order swapped: lane block
571571
// outer, row block inner, so one accumulator register carries eight lanes'
572572
// running sums down all 32 rows and never spills. Each 8x8 register block is
573-
// eight loads, eight adds along the chain, the same unpack ladder
573+
// eight loads, eight adds along the chain, the same shuffle sequence
574574
// Transpose32x32Avx2 uses, and eight 32-byte stores across a row of the output.
575575
// The adds are the only addition to the transpose's instruction stream, and they
576576
// sit on the load-to-shuffle path rather than after it.
@@ -658,7 +658,7 @@ inline void PrefixSumAndTransposeAvx2(const uint32_t* ARROW_RESTRICT grid,
658658
// of from memory, so each packed word is read once and each output value written
659659
// once, and the block never exists. Nothing else changes -- the accumulator
660660
// still carries eight lanes' running sums down all 32 rows in one register, and
661-
// the ladder is still the one Transpose32x32Avx2 uses.
661+
// the shuffle sequence is still the one Transpose32x32Avx2 uses.
662662
//
663663
// kHasBias is true here rather than folded into the accumulator: the wire format
664664
// stores min-subtracted deltas, so every one of the 32 values in a lane's chain

0 commit comments

Comments
 (0)