Re: [patch] i386: make bitops safe

[Chuck Ebbert <76306.1226@compuserve.com>]

In-Reply-To: <20060228005436.GA24895@redhat.com>

On Mon, 27 Feb 2006 at 16:54:36 -0800, Richard Henderson wrote:

> On Tue, Feb 28, 2006 at 12:47:22AM +0100, Andi Kleen wrote:
> > I remember asking rth about this at some point and IIRC
> > he expressed doubts if it would actually do what expected. Richard?
> 
> It's a bit dicey to be sure.  GCC may or may not be able to look
> through the size of the array and not kill things beyond it.  If
> one could be *sure* of some actual maximum index, this would be
> fine, but I don't think you can.
> 

In theory the bit offset could be from -2**31 to 2**31 - 1

> One could reasonably argue that if you used a structure with a
> flexible array member, that GCC could not look through that.  But
> again I'm not 100% positive this is handled properly.

This seems to work but causes more problems than it solves:

#define vaddr ((volatile long *) addr)
static inline void set_bit(int nr, volatile unsigned long * addr)
{
        __asm__ __volatile__( "lock ; "
                "btsl %2,%1"
                :"+m" (*(vaddr + (nr>>5)))
                :"m" (*vaddr),"Ir" (nr)
                );
}

First, it generates the byte offset nr>>5 and puts it in a register
even though it will never be used in the asm.  I can't find a constraint
that says "I'll be accessing this address but I don't need you to generate
it for me."  Second, the compiler thinks *vaddr will be read when it
really won't (unless nr>>5 == 0 in which case constraint 0 takes care
of it.)

Generated code when nr is a variable:

        movl nr,%edx
        movl %edx,%eax
        sarl $5,%eax
        sall $2,%eax
        lock ; btsl %edx,addr

This causes a register reload afterward (assuming all regs are busy) and
can cause a function to use more stack space.  That plus the three extra
instructions made me go with the full memory clobber instead.

-- 
Chuck
"Equations are the Devil's sentences."  --Stephen Colbert