Subject: Re: CVS commit: src/regress/sys/kern/ras
To: Martin Husemann <martin@duskware.de>
From: Stephan Uphoff <ups@stups.com>
List: source-changes
Date: 01/07/2004 23:25:27
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