MaskRay wrote: I understand the review cost and appreciate the work that went into the original algorithm #65742.
I wouldn't propose this refactoring purely for code structure. The motivation is practical: I need to ensure all bit-field memory accesses are 8 bytes (for a specific use case). This worked before #65742 changed how int64_t/uint64_t bit-fields are handled. After that change broke it, I spent many hours studying the algorithm to figure out a workaround (and then many hours to develop this refactoring). The interleaved loop made it genuinely difficult to understand where and how access unit sizing decisions are made. **The is still the same algorithm with the two logical phases made explicit. The output is unchanged.** Extracting some helpers would reduce `if/for` nesting but the structural complexity will remain, e.g. `InstallBest` set in 5 places. I believe no helper extraction will fix that without splitting the loop. That said, I'm happy to land the getLimitOffset extraction as a first step if you'd prefer an incremental approach. To be more concrete, these local variables are eliminated by this refactoring ``` AtAlignedBoundary (bool) Flag: true iff Field is the (potential) start of a new span (or the end of the bitfields). Drove the main control flow branch in the single loop. Eliminated because Pass 1 handles span detection explicitly. Barrier (bool, loop-scoped) Flag: true when the current boundary is a zero-width bitfield or non-bitfield, preventing merging with the next span. Eliminated because Pass 1 records this as the Barrier field in BitFieldSpan. InstallBest (bool) Flag: true when the loop should emit the best access unit and reset. Had 5 separate sites that set it to true. Eliminated because Pass 2 uses explicit break/continue control flow instead of flag-driven dispatch. BitSizeSinceBegin (uint64_t) Running bit count of the current accumulation, including padding when advanced past a subsequent bitfield run. Eliminated because Pass 2 computes AccessSize directly from span offsets and sizes. LimitOffset (CharUnits) Per-iteration limit offset (next field with storage or tail clipping). Eliminated as a variable; now computed on demand by the getLimitOffset lambda. Begin (RecordDecl::field_iterator, mutable) Start of the current span, mutated throughout the loop. Eliminated because Pass 2 indexes spans via Spans[I].Begin. ``` https://github.com/llvm/llvm-project/pull/182814 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
