Re: [Xen-devel] [PATCH] x86/HVM: slightly improve hvm_mmio_{first, last}_byte()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.04.16 at 10:41, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 26 April 2016 09:28
>> To: xen-devel
>> Cc: Paul Durrant; Wei Liu
>> Subject: [PATCH] x86/HVM: slightly improve hvm_mmio_{first,last}_byte()
>> 
>> EFLAGS.DF can be assumed to be usually clear, so unlikely()-annotate
>> the conditionals accordingly.
>> 
>> Also prefer latching p->size (used twice) into a local variable, at
> 
> Well, it should only be used once since only one of the expressions should 
> be evaluated.

But there would still be two references in code, and the trivial line
of thought here is that (leaving optimization aside) accessing some
structure field twice generate code no smaller (and possibly larger)
than accessing some other structure field just once.

>> once making it unnecessary for the reader to be sure expressions get
>> evaluated left to right (operand promotion would yield a different
>> result if p->addr + p->size - 1 was evaluted right to left).
> 
> Would that not be cured by replacing 1 with 1ul?

That's another possibility, but (being a matter of taste) I prefer to
avoid type suffixes.

Jan

>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> --- a/xen/include/asm-x86/hvm/io.h
>> +++ b/xen/include/asm-x86/hvm/io.h
>> @@ -44,18 +44,18 @@ struct hvm_mmio_ops {
>> 
>>  static inline paddr_t hvm_mmio_first_byte(const ioreq_t *p)
>>  {
>> -    return p->df ?
>> +    return unlikely(p->df) ?
>>             p->addr - (p->count - 1ul) * p->size :
>>             p->addr;
>>  }
>> 
>>  static inline paddr_t hvm_mmio_last_byte(const ioreq_t *p)
>>  {
>> -    unsigned long count = p->count;
>> +    unsigned long size = p->size;
>> 
>> -    return p->df ?
>> -           p->addr + p->size - 1:
>> -           p->addr + (count * p->size) - 1;
>> +    return unlikely(p->df) ?
>> +           p->addr + size - 1:
>> +           p->addr + (p->count * size) - 1;
>>  }
>> 
>>  typedef int (*portio_action_t)(
>> 
>> 




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel