On Wed, 1 Aug 2007, Paul Mackerras wrote:
>
> It will mean more code on architectures which have a
> conditional-trap-on-nonzero instruction, such as powerpc, since the
> compiler will generate instructions to evaluate !!x. But I don't see
> any reason why ret_warn_on couldn't be a long.
Umm. The WARN_ON() might actually get a "long long" value for all we know.
Ie it's perfectly possible that the WARN_ON might look like
/* Must not have high bits on */
WARN_ON(offset & 0xffffffff00000000);
which on a 32-bit pcc would apparently do the wrong thing entirely as it
stands now. No?
I think I'll commit the !!(x) version, and you guys can try to figure out
what the right thing is long-term. For all I know, the proper solution is
to just revert the whole mess, and *not* make WARN_ON() return a value at
all, since that seems to be the fundamental mistake here.
Linus
On Wed, Aug 01, 2007 at 02:20:48PM +1000, Paul Mackerras wrote:
> Linus Torvalds writes:
>
> > Umm. The WARN_ON() might actually get a "long long" value for all we know.
> > Ie it's perfectly possible that the WARN_ON might look like
> >
> > /* Must not have high bits on */
> > WARN_ON(offset & 0xffffffff00000000);
> >
> > which on a 32-bit pcc would apparently do the wrong thing entirely as it
> > stands now. No?
>
> Actually, because of the typeof in the powerpc WARN_ON, I think it
> would fail to build since we'd be passing a long long value to an
> inline asm, or at least I hope it would fail to build. :)
Turning the condition into an integer should work ...
#define NEW_WARN_ON(x) OLD_WARN_ON(!!(x))
Regards,
Joe
Linus Torvalds writes:
> Umm. The WARN_ON() might actually get a "long long" value for all we know.
> Ie it's perfectly possible that the WARN_ON might look like
>
> /* Must not have high bits on */
> WARN_ON(offset & 0xffffffff00000000);
>
> which on a 32-bit pcc would apparently do the wrong thing entirely as it
> stands now. No?
Actually, because of the typeof in the powerpc WARN_ON, I think it
would fail to build since we'd be passing a long long value to an
inline asm, or at least I hope it would fail to build. :)
But your criticism is correct with regard to the powerpc BUG_ON, and
you're correct that a long wouldn't be sufficient if someone passes in
a long long. Oh well. I guess we just wear the extra two
instructions.
Paul.