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)) \
))
In a lot of the code in Xen we are using the "!" operator with bitwise
operations..this is one of those examples.
--
Jerone Young
IBM Linux Technology Center
jyoung5@us.ibm.com
512-838-1157 (T/L: 678-1157)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
lists.xensource.com/xen-devel
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 "!"? > >The usual use of !!a is to do (a != 0) ? 1 : 0. I''m not sure if there''s a more readable way to do it. Regards, Anthony Liguori>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)) \ > )) > > >In a lot of the code in Xen we are using the "!" operator with bitwise >operations..this is one of those examples. > > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
On 11 Aug 2005, at 16:55, Jerone Young wrote:> the part where we have !!current->vcpu_info_evtchen_upcall pending > should this be..should the "!! just be "!"?The code is being a bit defensive, and dealing with the case that evtchn_upcall_pending may be non-zero, but the least significant bit isn''t set. That is never actually the case (Xen never sets any other bit than the lsb) so the code could be changed, but not in the way you suggest. The correct change would be simply to remove the !!.> In a lot of the code in Xen we are using the "!" operator with bitwise > operations..this is one of those examples.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. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
>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 "!"?I think that: !!a & !b === (a != 0 ) && ( b == 0 ) It''s just more efficient to do it directly using bitwise ops than to use the full logical operators (can gcc really not optimise this sort of thing???). Cheers, Mark>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)) \ > )) > > >In a lot of the code in Xen we are using the "!" operator with bitwise >operations..this is one of those examples. > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
On 11 Aug 2005, at 18:03, M.A. Williamson wrote:> I think that: > !!a & !b === (a != 0 ) && ( b == 0 ) > > It''s just more efficient to do it directly using bitwise ops than to > use the full logical operators (can gcc really not optimise this sort > of thing???).It can''t optimise out short-circuit semantics. Without predicates on instructions (like e.g. ARM has) it has to emit conditional branches. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
On Thu, 11 Aug 2005, Keir Fraser 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.Is it worth it to perform these optimizations? For example the code to get time information in xen/386 linux: do { shadow_tv_version = s->wc_version; rmb(); shadow_tv.tv_sec = s->wc_sec; shadow_tv.tv_nsec = s->wc_nsec; rmb(); } while ((s->wc_version & 1) | (shadow_tv_version ^ s->wc_version)); Granted most experienced programmers will recognize that the xor is used to test for inequality and the bitwise-or is used as a logical-or. But how often does this piece of code even run? Seems like a case of trading readability for speed when optimization isn''t even important.> -- KeirTim Newsham lava.net/~newsham _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser wrote:> On 11 Aug 2005, at 18:03, M.A. Williamson wrote: > >> I think that: >> !!a & !b === (a != 0 ) && ( b == 0 ) >> >> It''s just more efficient to do it directly using bitwise ops than to >> use the full logical operators (can gcc really not optimise this sort >> of thing???). > > It can''t optimise out short-circuit semantics.Yes it can, if the second branch of the short-circuit is guaranteed to terminate with no side effects when it has defined behaviour. The function thread_jump in <gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cfgcleanup.c?rev=1.140.2.7> attempts to prove that a basic block satisfies this property, although I''m not sure whether gcc is capable of doing the specific optimization we''re talking about here. I know that some other C compilers are. -- David Hopwood <david.nospam.hopwood@blueyonder.co.uk> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Seemingly Similar Threads
- evtchn_upcall_mask
- [PATCH v5 0/5] xen: public interface and foreign struct check changes for arm
- [PATCH] x86/hvm: don't give vector callback higher priority than NMI/MCE
- evtchn_upcall_mask for PV-on-HVM
- [PATCH] xen: Use wmb instead of rmb in xen_evtchn_do_upcall().