RFR: 8352635: Improve inferencing of Float16 operations with constant inputs [v5]

[Emanuel Peter]

On Wed, 4 Jun 2025 10:47:57 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> src/hotspot/share/opto/convertnode.cpp line 294:
>> 
>>> 292:       // Conditions under which floating point constant can be considered for a pattern match.
>>> 293:       // 1. Constant must lie within Float16 value range, this will ensure that
>>> 294:       // we don't unintentially round off float constant to enforce a pattern match.
>> 
>> What do you mean by `enforce a pattern match`?
>> 
>> Are you just trying to say that we have to be careful with the pattern matching here, and we cannot just round off the float constant? Do you have an example where that rounding would lead to issues?
>
> import jdk.incubator.vector.*;
> public class verify_rounding {
>    public static void check() {
>        for (int i = 0; i < 65550; i++) {
>            short post_rounding = Float.floatToFloat16(Float.float16ToFloat(Float.floatToFloat16((float)i)) * 2049.0f);
>            short pre_rounding = Float16.float16ToRawShortBits(Float16.multiply(Float16.valueOf((float)i), Float16.valueOf((float)2049.0f)));
>            if (pre_rounding != post_rounding) {
>               System.out.println("Mismatch at val = " + (float)i);
>               System.out.println("post_rounding val = " + post_rounding);
>               System.out.println("pre_rounding val = " + pre_rounding);
>               break;
>            }
>        }
>    }
> 
>    public static void main(String [] args) {
>       check();
>    }
> }
> 
> 
> CPROMPT>java --add-modules=jdk.incubator.vector -cp . verify_rounding
> WARNING: Using incubator modules: jdk.incubator.vector
> Mismatch at val = 3.0
> post_rounding val = 28161
> pre_rounding val = 28160
> 
> Since we intend to infer Float16 IR using patten match,  hence it may be incorrect to transform post-rounding pattern to pre-rounding.

Right ok. The wording `enforce a pattern match` still does not make sense to me. You can `perform` a `pattern match`, but what does it mean to `enforce` it? Can you rephrase please?

>> test/hotspot/jtreg/compiler/c2/irTests/TestFloat16ScalarOperations.java line 335:
>> 
>>> 333:         res += Float.floatToFloat16(POSITIVE_ZERO_VAR.floatValue() - INEXACT_FP16);
>>> 334:         res += Float.floatToFloat16(INEXACT_FP16 * POSITIVE_ZERO_VAR.floatValue());
>>> 335:         res += Float.floatToFloat16(POSITIVE_ZERO_VAR.floatValue() / INEXACT_FP16);
>> 
>> Why is the mul case flipped here?
>
> To check for constant on either side of an expression.

I see. It looks a little strange. You could leave a comment for the reader.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24179#discussion_r2140288570
PR Review Comment: https://git.openjdk.org/jdk/pull/24179#discussion_r2140306802