Makes sense, thanks for the quick reply. Last question, the patches can depend on others in the same set right? IE: all of the additions to insn-data.def in one, implementations in separate patches.
Thanks - David Miller On Thu, Mar 3, 2022 at 12:42 PM Richard Henderson < richard.hender...@linaro.org> wrote: > On 3/3/22 06:50, David Miller wrote: > > > > > Too many changes in one patch. > > > You need to split these into smaller, logical units. > > > > Can you give some guideline on that? > > IE: change to two, the shifts and reversed loads into two patches or > more on line count > > of each patch? > > Your best guide is line count: < 50 is ideal, though of course that can't > always be done. > For bug fixes or code reorg you may find yourself constrained by not > breaking bisection. > > But for new code, like this, one patch per feature is easiest to review. > In this case > you've got: > > - load/store elements reversed, > - load/store byte reversed elements, > - shift double > - string search > - modify fp convert > - modify shift > > > I wasn't sure if there was a reason MO_TE was used so just kept with the > existing code flow. > > We have to put some indication of endianness there, and "target" endian > was the easiest to > replicate across all targets. Especially with those that are bi-endian. > > I've just noticed that we haven't propagated this to the integer > load/store reversed. I > presume that code pre-dates the existence of the feature. But it would be > good to change > > C(0xe31f, LRVH, RXY_a, Z, 0, m2_16u, new, r1_16, rev16, 0) > C(0xe31e, LRV, RXY_a, Z, 0, m2_32u, new, r1_32, rev32, 0) > C(0xe30f, LRVG, RXY_a, Z, 0, m2_64, r1, 0, rev64, 0) > ... > C(0xe33f, STRVH, RXY_a, Z, la2, r1_16u, new, m1_16, rev16, 0) > C(0xe33e, STRV, RXY_a, Z, la2, r1_32u, new, m1_32, rev32, 0) > C(0xe32f, STRVG, RXY_a, Z, la2, r1_o, new, m1_64, rev64, 0) > > to use little-endian memory ops, rather than separately reversing the > bytes. > > > r~ >
