linux-mips
[Top] [All Lists]

Re: [PATCH] Put cast inside macro instead of all the callers

To: Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH] Put cast inside macro instead of all the callers
From: "Maciej W. Rozycki" <macro@linux-mips.org>
Date: Thu, 8 Nov 2007 15:26:26 +0000 (GMT)
Cc: Ulrich Eckhardt <eckhardt@satorlaser.com>, linux-mips <linux-mips@linux-mips.org>
In-reply-to: <20071101174705.GA23917@linux-mips.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20071031141124.185599da@ripper.onstor.net> <200711011704.01079.eckhardt@satorlaser.com> <20071101174705.GA23917@linux-mips.org>
Sender: linux-mips-bounce@linux-mips.org
On Thu, 1 Nov 2007, Ralf Baechle wrote:

> > I'm not sure if this is always a compile-time constant so that you can adorn
> > it with a LL. However, note that this is not a cast, a cast is at runtime.
> 
> No.  The compiler can evaluate the cast of a constant value at compile
> time and that exactly is what the code is exploiting here.

 Except that some versions of GCC "forget" to stop a warning here as 
irrelevant after the cast, hence the need for the stupid "#ifdef", sigh...

> > >   if (sp >= (long)CKSEG0 && sp < (long)CKSEG2)
> > >           usp = CKSEG1ADDR(sp);
> > >  #ifdef CONFIG_64BIT
> > > - else if ((long long)sp >= (long long)PHYS_TO_XKPHYS(0LL, 0) &&
> > > -          (long long)sp < (long long)PHYS_TO_XKPHYS(8LL, 0))
> > > -         usp = PHYS_TO_XKPHYS((long long)K_CALG_UNCACHED,
> > > + else if ((long long)sp >= (long long)PHYS_TO_XKPHYS(0, 0) &&
> > > +          (long long)sp < (long long)PHYS_TO_XKPHYS(8, 0))
> > > +         usp = PHYS_TO_XKPHYS(K_CALG_UNCACHED,
> > >                                XKPHYS_TO_PHYS((long long)sp));
> > 
> > I'd say this code is broken in way too many aspects:
> > 1. A plethora of casts. PHYS_TO_XKPHYS() should return a physical address 
> > (i.e. 32 or 64 bits unsigned integer) already, so casting its result should 
> > not be necessary.
> 
> No argument about the beauty of the whole thing.

 The casts were an attempt by myself to shut up GCC warning about 
comparisons giving a predictable result because of a limited size of the 
data type used.  Of course this is no longer true with "long long", but 
GCC does not care and warns regardless.

 A possible workaround would be using auxiliary variables of the "long 
long" type, but I recall it making GCC produce worse code for some cases.  
I can check it again, since it has been a while, and if a recent version 
of GCC produces reasonable code, then I can try to recheck this approach.

 Please note this code changes quite wildly depending on the exact 
configuration, so chances are GCC may warn with one, but not another.

  Maciej

<Prev in Thread] Current Thread [Next in Thread>