From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Hopwood Subject: Re: code question? Date: Thu, 11 Aug 2005 17:55:53 +0100 Message-ID: <42FB8319.8090706@blueyonder.co.uk> References: <1123775716.3043.15.camel@thinkpad> Reply-To: david.nospam.hopwood@blueyonder.co.uk Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1123775716.3043.15.camel@thinkpad> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org 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