Re: code question?

[David Hopwood <david.nospam.hopwood@blueyonder.co.uk>]

Jerone Young wrote:
> Doing some janitorial (you cleaning the flooded toilets and such) work
> today. I have come across this line of code that really I'm not sure
> what the intent was..in xen/include/sched.h
> 
> #define hypercall_preempt_check() (unlikely(            \
>         softirq_pending(smp_processor_id()) |           \
>         (!!current->vcpu_info->evtchn_upcall_pending &  \
>           !current->vcpu_info->evtchn_upcall_mask)      \
>     ))
> 
> the part where we have !!current->vcpu_info_evtchen_upcall pending
> should this be..should the "!! just be "!"?
> 
> And if so shouldn't this just be changed to 
> 
> #define hypercall_preempt_check() (unlikely( \
> 	softirq_pending(smp_processor_id()) |           \
> 	(!(current->vcpu_info->evtchn_upcall_pending & \
> 	  current->vcpu_info->evtchn_upcall_mask)) \
> 	))

Seems more likely to have been intended as:

   #define hypercall_preempt_check() (unlikely(           \
           softirq_pending(smp_processor_id()) ||         \
           (current->vcpu_info->evtchn_upcall_pending &&  \
             !current->vcpu_info->evtchn_upcall_mask)     \
       ))

Keir wrote:
 > Forming compound predicates for bitwise operators can be faster than using the
 > logical operators because they are non 'short circuiting'. This means you end
 > up with fewer branches in the generated code and end up with nice straight-line
 > code that should execute fast.

If that's true on a given platform, the compiler can optimize it. Note that
in this case "(current->vcpu_info->evtchn_upcall_pending &&
!current->vcpu_info->evtchn_upcall_mask)" has no side effects (unless current
or current->vcpu_info are not valid pointers, which is undefined behaviour so
still optimizable).

-- 
David Hopwood <david.nospam.hopwood@blueyonder.co.uk>