Re: [Xen-devel] code question?

[David Hopwood <david.nospam.hopwood@xxxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxx>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel