RFR: 8266530: HotSpot changes for JEP 306 [v3]

[David Holmes]

Hi Joe,

On 12/05/2021 4:11 am, Joe Darcy wrote:
> On 5/10/2021 5:20 PM, David Holmes wrote:
>> On 11/05/2021 5:14 am, Harold Seigel wrote:
>>> On Fri, 7 May 2021 04:58:22 GMT, Joe Darcy <darcy at openjdk.org> wrote:
>>>
>>>>> Please review these VM changes in support of JEP 306: Restore 
>>>>> Always-Strict Floating-Point Semantics.
>>>>>
>>>>> JEP 306 is considering the following changes to the JVMS:
>>>>>
>>>>> 1) Require "strict" floating-point evaluation in all circumstances.
>>>>> 2) For class file version 61 and higher, undefine ACC_STRICT, 
>>>>> moving that bit position back to undefined for methods. For class 
>>>>> file versions, 46 through, ACC_STRICT would remain a defined modifier.
>>>>>
>>>>> Since all floating-point evaluation is done strictly, a modifier 
>>>>> bit is not needed to indicate this anymore. There are a few 
>>>>> mandated modifier checks when ACC_STRICT is defined, such as not 
>>>>> allowing an "abstract strictfp" method. These checks in 
>>>>> classFileParser are now conditional on the class file version.
>>>>>
>>>>> The tests verify  "abstract strictfp" triggers a ClassFormatError 
>>>>> for version 60 and does not trigger an error or exception for 
>>>>> version 61. (As an undefined bit in version 61, the VM is supposed 
>>>>> to ignore it.)
>>>>>
>>>>> FYI, the javac and core libs portions of JEP 306 are out for review 
>>>>> under https://github.com/openjdk/jdk/pull/3831.
>>>>>
>>>>> The HotSpot and javac/core libs parts of JEP 306 are independent in 
>>>>> that neither relies on the other:
>>>>>   * javac will not output class files which expose the difference 
>>>>> in class parsing behavior; the ACC_STRICT modifier will not be set 
>>>>> in any version 61 class file emitted by javac.
>>>>>   *  since HotSpot already does all floating-point evaluation 
>>>>> strictly, no changes are needed to support strict evaluation.
>>>>
>>>> Joe Darcy has updated the pull request with a new target base due to 
>>>> a merge or a rebase. The incremental webrev excludes the unrelated 
>>>> changes brought in by the merge/rebase. The pull request contains 
>>>> three additional commits since the last revision:
>>>>
>>>>   - Merge branch 'master' into 8266530
>>>>   - Respond to review feedback.
>>>>   - 8266530: HotSpot changes for JEP 306
>>>
>>> Hi Joe,
>>> If the code that calls is_strict() (and other code that looks at 
>>> ACC_STRICT) is no longer needed then it should be removed. Otherwise, 
>>> that code should be changed to also check if ik->major_version() >= 61.
>>
>> Right - the normal process would be to update the code based on 
>> is_strict() always returning true thus eliding it, but retaining the 
>> code it causes to be called.
>>
> I've pushed another changeset which defines is_strict in method.hpp as 
> true under IA32. If IA32 covers all platforms that could use the x87 FPU 
> stack, this should be sufficient for correctness.

I'm not seeing that changeset in the PR ??

But it sounds insufficient. Every expression using is_strict() should 
replace is_strict() with true and then collapse the code appropriately.

Cheers,
David

> Thanks,
> 
> -Joe
>