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
|