RFR: 8319451: PhaseIdealLoop::conditional_move is too conservative

[Claes Redestad]

On Mon, 6 Nov 2023 19:10:42 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> When transforming a Phi into a CMove, the threshold is set to be approximately BlockLayoutMinDiamondPercentage, the reason is given:
> 
>     // BlockLayoutByFrequency optimization moves infrequent branch
>     // from hot path. No point in CMOV'ing in such case
> 
> This sets the default value of the threshold to be around 18%, which is too conservative. The reason also does not make a lot of sense since the important property which makes jumping expensive is not code layout. We should remove this.
> 
> Please kindly review, thank you very much.

Nice, especially the property that run-to-run variance drops in the 1-20% range. 

@TobiHartmann told me he'll review and that he has queued up a batch of benchmarks to evaluate this change more widely.

test/micro/org/openjdk/bench/vm/compiler/CMove.java line 40:

> 38: 
> 39:     @Param({"3", "6", "10", "20", "30", "60", "100", "200", "300", "600"})
> 40:     int freq;

That `freq` is expressed in "occurrences per thousand" was only obvious after reading the code - perhaps a probability between 0 and 1 (with the appropriate adjustment to `r.nextFloat() < freq` below) would be slightly more intuitive?
Suggestion:

    @Param({"0.003", "0.006", "0.01", "0.02", "0.03", "0.06", "0.1", "0.2", "0.3", "0.6"})
    float freq;

-------------

PR Review: https://git.openjdk.org/jdk/pull/16524#pullrequestreview-1722679005
PR Review Comment: https://git.openjdk.org/jdk/pull/16524#discussion_r1388087404