On 06/10/15 12:53, Chris Wilson wrote: > On Tue, Oct 06, 2015 at 12:21:05PM +0100, Dave Gordon wrote: >> ... although I still think my version is (slightly) easier to read. >> Or it could be improved even more by moving the increment of 'iter' >> to the end, making it one line shorter and perhaps helping the >> compiler a little :) >> >> #define gen8_for_each_pml4e(pdp, pml4, start, length, temp, iter) \ >> for (iter = gen8_pml4e_index(start); \ >> iter < GEN8_PML4ES_PER_PML4 && \ >> (pdp = (pml4)->pdps[iter], length > 0); \ >> temp = ALIGN(start+1, 1ULL << GEN8_PML4E_SHIFT) - start, \ >> temp = min(temp, length), \ >> start += temp, length -= temp, ++iter) > > Shorter, yes, but we may as well take advantage of not using [iter] if > length == 0, so meh. > >> Or, noting that 'temp' is never used in the generated loop bodies, >> we could eliminate the parameter and make it local to the loop >> header :) >> >> #define gen8_for_each_pml4e(pdp, pml4, start, length, iter) \ >> for (iter = gen8_pml4e_index(start); \ >> iter < GEN8_PML4ES_PER_PML4 && \ >> (pdp = (pml4)->pdps[iter], length > 0); \ >> ({ u64 temp = ALIGN(start+1, 1ULL << GEN8_PML4E_SHIFT); \ >> temp = min(temp - start, length); \ >> start += temp, length -= temp; }), \ >> ++iter) > > Removing extraneous parameters from macros is differently a usability win. > Care to spin a real patch so we can see how it looks in practice? > -Chris OK, here's the patch that Chris asked for. On compilation, this version comes out about 160 bytes smaller than the original, and the disassembly has 100 fewer lines. This probably doesn't improve performance significantly, though, as many of the uses of these macros are inside debug-only data dumping functions. .Dave.