Re: [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.03.2020 13:31, Julien Grall wrote:
> On 18/03/2020 09:56, Jan Beulich wrote:
>> On 17.03.2020 22:52, David Woodhouse wrote:
>>> On Thu, 2020-02-20 at 12:10 +0100, Jan Beulich wrote:
>>>> On 07.02.2020 16:57, David Woodhouse wrote:
>>>>> +#define page_is_offlining(pg)      (page_state_is((pg), 
>>>>> broken_offlining) || \
>>>>> +                                    page_state_is((pg), offlining))
>>>>
>>>> Overall I wonder whether the PGC_state_* ordering couldn't be
>>>> adjusted such that at least some of these three won't need
>>>> two comparisons (by masking off a bit before comparing).
>>>
>>> The whole point in this exercise is that there isn't a whole bit for
>>> these; they are each *two* states out of the possible 8.
>>
>> Sure. But just consider the more general case: Instead of writing
>>
>>      if ( i == 6 || i == 7 )
>>
>> you can as well write
>>
>>      if ( (i | 1) == 7 )
> 
> I stumbled accross a few of those recently and this is not the
> obvious things to read.

Depends on the reader, I guess.

> Yes, your code may be faster. But is it really worth it compare
> to the cost of readability and futureproofness?

Not sure how being future proof comes into play here. I'm not
suggesting to use literal numbers. I'd also be happy to be proven
wrong in assuming that the compiler still can't do such
transformations itself; it couldn't when I check a while back.
Reducing the number of conditional branches is, imo, worth at
least some effort at the source level.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel