Don, could you add this to the bug report, please?

On 8/23/2011 5:46 AM, Don Clugston wrote:
>
>>> The backend does spill the x87 stack when it overflows. I suspect the problem may lie in the scheduler. This is easily checked by disabling the scheduler and trying it.
>> You're right, it's the scheduler. It loads 8 values on the x87 stack,
>> then does fdecstp.
>> Then, the 9th load lands on top of an existing value, causing a stack
>> overflow exception. The loaded value gets turned into a NaN.
>> I'll try to track it down further.
> OK, here's the problem. The scheduler keeps track of the number of
> used x87 registers via the "fpustackused" variable.
> Each instruction has a "fpuadjust" value which says how many x87
> registers it uses.
> But, the problem is CALL instructions. They have fpuadjust = 0.
> But, a function returning float returns it in ST(0) --- so fpuadjust
> should be +1.
> And if it's a complex result, fpuadjust should be +2.
> I think it should be possible to put an assert in cgsched.c, line 2491:
>
>              if (tbl[j])
>              {
>                  fpu += tbl[j]->fpuadjust;
> + assert(fpu>= 0);
>                  if (fpu>= 8)                   // if FPU stack overflow
>
> but this would fail at the moment, because the total goes negative
> after a call followed by an FSTP.
> So it needs to distinguish between each type of call, depending on how
> it affects the FPU stack.
> I think the bug lies in funccall() in cod1.c; it should be setting
> something in 'code' to record how many x87 registers are returned.
>
> Then, there'll be another minor problem: the number of FPU values CAN
> reach 8. This is because of the code in push87() in cg87.c, which does
> a
> fdecstp; fstp; when the stack is full. So the condition above will
> need to change to:
>                  if (fpu>  8)                   // if FPU stack overflow
>
> This is too invasive a change for me to make and test.
>

On Aug 23, 2011, at 12:55 PM, Brad Roberts <braddr at puremagic.com> wrote:
> 
> For the time being, since this has been broken for a long time, can we revert/disable the failing test so that it doesn't hide other failures?

Personally, I hate the idea of completely disabling a test. I like when test suites can handle expected failures or ignored tests. As an example, cruise control will use yellow for ignored test, red for failed tests, and green for passing tests. That's nicer, but still annoying when trying to do test driven development.

On Tue, 23 Aug 2011, Jason House wrote:
> On Aug 23, 2011, at 12:55 PM, Brad Roberts <braddr at puremagic.com> wrote:
> > 
> > For the time being, since this has been broken for a long time, can we revert/disable the failing test so that it doesn't hide other failures?
> 
> Personally, I hate the idea of completely disabling a test. I like when test suites can handle expected failures or ignored tests. As an example, cruise control will use yellow for ignored test, red for failed tests, and green for passing tests. That's nicer, but still annoying when trying to do test driven development.

In general, I greatly dislike disabling tests.  That said, when it's an OLD failure checking in the broken test before there's a fix for it, the value is questionable.

Additionally, the behavior your advocating is not what we have currently, so using what we have currently the best way we can is better than some theoretical it could be done differently.

If you'd like to invest in improving the infrastructure, please feel encouraged to do so.

Later,
Brad