public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Andi Kleen <andi@firstfloor.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Eliminate result signage problem in asm-x86_64/bitops.h
Date: Wed, 15 Aug 2007 18:42:30 -0400	[thread overview]
Message-ID: <46C38156.8060502@oracle.com> (raw)
In-Reply-To: <20070815222355.GA5069@one.firstfloor.org>

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]

I apologize for sending a separate cover letter for a single patch.

Andi Kleen wrote:
> On Wed, Aug 15, 2007 at 05:02:47PM -0400, Chuck Lever wrote:
>> The return type of __scanbit() doesn't match the return type of
>> find_{first,next}_bit().  Thus when you construct something like
>> this:
>>
>>    boolean ? __scanbit() : find_first_bit()
> 
> Why would you want to write this?  What is boolean?
> Do they have different arguments?

So here's the definition of the x86_64 find_first_bit() macro, straight 
from include/x86_64/bitops.h:

#define find_first_bit(addr,size) \
((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \
   (__scanbit(*(unsigned long *)addr,(size))) : \
   find_first_bit(addr,size)))

In this case "boolean" is:

    __builtin_constant_p(size) && (size) <= BITS_PER_LONG

the first arm of the conditional is:

    __scanbit(*(unsigned long *)addr,(size))

the second arm of the conditional is:

    find_first_bit(addr,size)

(this is the "function" version of find_first_bit, not the macro that's 
being defined.  The naming here is unfortunately confusing).

Thus, roughly speaking, when the type of "size" is smaller than a long, 
the macro's return type evaluates to unsigned long.  If "size" is larger 
than a long, the macro's return type evaluates to signed long.

By making the return type of __scanbit() an unsigned long, both arms of 
the conditional evaluate to the same result type.

> It's on my todo list for some time to special case
> f_f_b() and friends for smaller arguments. Would
> that eliminate this construct?

Well, I can only assume what you mean by this, but I think that would 
address the problem.

My real interest here is to eliminate a whole lot of compiler noise when 
I enable -Wsign-compare for certain parts of the kernel.

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 290 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard


      reply	other threads:[~2007-08-15 22:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-15 21:02 [PATCH] Fix the sign of the result of a conditional expression Chuck Lever
2007-08-15 21:02 ` [PATCH] Eliminate result signage problem in asm-x86_64/bitops.h Chuck Lever
2007-08-15 22:23   ` Andi Kleen
2007-08-15 22:42     ` Chuck Lever [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46C38156.8060502@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox