RFR: 8334629: [BACKOUT] PhaseIdealLoop::conditional_move is too conservative [v2]
Jasmine Karthikeyan
jkarthikeyan at openjdk.org
Thu Jun 20 13:58:13 UTC 2024
On Tue, 11 Jun 2024 17:35:32 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:
>> Hi,
>>
>> I cannot explain the regression, comparing the current mainline to JDK-21 reveals a decrease in performance, yet it is only for some combinations of OS-GC and perfasm shows that the hot regions (>99% of execution time) do not contain differences that can explain the results.
>>
>> Consequently, with the advice of @TobiHartmann , I propose to effectively revert JDK-8319451 for the generation of `CMove`s inside loops. For those outside, the before-JDK-8319451 probability threshold is 0.001 and the current value is 0.01. I think the current value is more reasonable as evidenced by the benchmark added in JDK-8319451.
>>
>> Please kindly review, thank you very much.
>> Quan Anh
>
> Quan Anh Mai has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>
> Revert "8319451: PhaseIdealLoop::conditional_move is too conservative"
>
> This reverts commit ac968c36d7cc2e13270d28c9310178f6b654d7dc.
Ah yep, looking at the log the main IfNode has `P=0.891189`, so it doesn't succeed the new check `prob > 0.18 && prob < 0.82`. I think this is fundementally the same problem as https://github.com/openjdk/jdk/pull/18734, and increasing the number of predetermined elements ([here](https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/c2/irTests/TestIfMinMax.java#L184-L194)) should fix this. If it's important to get this patch in quickly though feel free to problemlist the test or disable the IR verification, I can take a look at it soon.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19650#issuecomment-2180772571
More information about the hotspot-compiler-dev
mailing list