RFR 8079562: Crash in C2 compiled code with STATUS_FLOAT_MULTIPLE_TRAPS

[David Holmes]

Hi Coleen,

On 3/08/2016 11:16 AM, Coleen Phillimore wrote:
>
>
> On 8/2/16 8:35 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 3/08/2016 8:34 AM, Coleen Phillimore wrote:
>>> Summary: Add AlwaysRestoreFPU and move the test to jtreg.
>>> Contributed-by: myself and christian.tornqvist at oracle.com
>>>
>>> Christian moved the test, I did some cleanup and added AlwaysRestoreFPU
>>> to it.
>>
>> You modified the original C code such that this comment is no longer
>> valid:
>>
>>   33 // Only valid for specific x86 based systems: linux-x86, or else
>> x86 with fpu_control.h
>
> Thanks, David.  I took out the rest of the comments but missed this one.
>>
>> Not sure why you did that - we were running this test on more than
>> just Windows, despite the naming.
>
> Actually, the #ifdefs were intended to only run on 32 bit linux as well
> as windows, but I don't think the ifdefs were correct, so the test
> always printed that it was skipped on platforms other than windows.  The
> test is only useful on windows.   Modifying the fpcw didn't have an
> effect on linux x86.  Seems silly to run the test on platforms where
> it's not testing anything.

It's been a while but I've been involved with this test in the past and 
I thought it was doing similar testing on other x86 platforms. As you 
say the main point is to verify that messing with the FPU control word 
doesn't break anything so I'm not sure that is a reason not to have such 
a test run on linux.

>>
>> I'm also unclear exactly what changed such that we need to add
>> AlwaysRestoreFPU ??
>
> https://bugs.openjdk.java.net/browse/JDK-8076284 allegedly changed the
> behavior in 2015.

I certainly can't make any connection between that bug and handling of 
the FPU control word. :(

> Setting floating point control word is something that we don't promise
> to work with generated code or the jvm code, which is why this
> -XX:+AlwaysRestoreFPU flag was added.

Understood, just puzzled about why this test suddenly requires it. Given 
the FPU mode used by the VM has not changed, why does the test now fail 
without this??

>>
>> Can you add @Summary info to the Java test (and/or other commentary)
>> as it is far from clear exactly what this is testing.
>>
>
> Yes, I can do that.   How about:
>
>  * @summary Test that modifying the floating point control word doesn't
> cause unhandled Windows floating point exceptions

That's fine -thanks. Can you also document with a comment exactly what 
our modification of the FPU control word is intended to do and how that 
relates to the normal FPU mode used by the VM. This is something I 
always have to go looking for :)

>
> Can summary lines be > 1 line or do they have to be one line?

A tag runs until the next tag is encountered so the summary can span 
multiple lines.

Thanks,
David

> Thanks,
> Coleen
>
>
>> Thanks,
>> David
>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8079562.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8079562
>>>
>>> Tested with JPRT.
>>>
>>> thanks,
>>> Coleen
>