Re: CVS commit: src/regress/sys/kern/ras

[Stephan Uphoff <ups%stups.com@localhost>]

Martin Husemann wrote:
> On Tue, Jan 06, 2004 at 08:37:41PM -0800, Jason Thorpe wrote:
> > Using:
> > 
> >     __asm __volatile("":::"memory");
> > 
> > on both sides of the RAS ought to be enough, if the RAS doesn't operate 
> > on any locals.
> 
> Ok, so I tested it and it works.
> What actually happens is that in the tests there is code like:
> 
>   if (rasctl(&&start, &&end, ...) < 0) fail...
> start:
>   /* RAS starts here */
> end:
>   /* test results */
> 
> Inside the RAS there are references to global objects, and the compiler moves
> the address load for one of the objects into the branch delay slot of the if,
> and uses the address after the delay slot as "&&start".
> ....
> Anyway, this is just one case of optimzation interfering with the code, 
> as Steve said we can not be sure w/o the big hammer (or the next gcc update
> may break it).
> 
> With the current compiler either Jasons memory clobber or Pauls instruction
> barrier before the RAS work. Thinking about it IMHO the global memory clobber
> is what we realy want here, as it exactly matches the RAS semantics.


Even big hammers can sometimes miss. (as my thumb remembers all to well :-)

Small changes like adding "count=0" just before the start label might have
the same effect as moving the load to the delayed branch delay slot.
( With or without memory clobber)

I would also consider it perfectly legal for future gcc releases to keep the 
address load in the branch delay slot despite the  memory clobber since 
it only involves a constant and a register (right?).

It is also possible  that the line "itv.it_value.tv_sec = 10;" puts
the value "10" in a register that is later used for the comparison
"if (count > COUNT)". ( COUNT is defined as 10)
Since the value is no longer used after the comparison the register can 
be clobbered in:
        while (!handled) {
                continue;
        }


Don't get me wrong - I am not advocating to change every ras section
to assembly code. (Or at least I am definitely not volunteering :-)

However a big warning to users looking for "cookbook recipes" on how
to use ras or future maintainers would be nice.

> It probably would not do that if the label "start:" had a goto using it. So
> this might be considered a bug in gcc (since taking the address of the label
> should have the same effect).

I guess taking the address of the label just makes "start:" the 
potential destination of all computed gotos.
Since there are no computed gotos in the test function this leaves 
the basic block starting with "start:" with only one predecessor.

        Stephan