On Thu, 1 Sep 2022 23:22:46 GMT, Smita Kamath <svkam...@openjdk.org> wrote:
>> 8289552: Make intrinsic conversions between bit representations of half >> precision values and floats > > Smita Kamath has updated the pull request incrementally with one additional > commit since the last revision: > > Added missing parantheses Some minor naming/formatting comments. Rest of the patch looks good to me. src/hotspot/share/runtime/sharedRuntime.cpp line 487: > 485: jshort signif_bits = (jshort)(f_signif_bits >> (13 + exp_delta)); > 486: > 487: jint lsb = (f_signif_bits & (1 << 13 + exp_delta)); It will be more clear to have the parenthesis like this: jint lsb = f_signif_bits & (1 << (13 + exp_delta)); src/hotspot/share/runtime/sharedRuntime.cpp line 522: > 520: > 521: // Add the bias of float exponent and shift > 522: int float_exp_bits = ((hf_exp + 127) << 24 - 1); It will be more clear to have the parenthesis like this: int float_exp_bits = (hf_exp + 127) << (24 - 1); May be we should refer to src/java.base/share/classes/java/lang/Float.java:floatToFloat16 and Float16toFloat in comments. src/hotspot/share/runtime/sharedRuntime.cpp line 525: > 523: > 524: // Combine sign, exponent and significand bits > 525: return (jfloat) ((hf_sign_bit << 16) | float_exp_bits | > (hf_significand_bits << significand_shift)); You could call SharedRuntime::i2f here as well to be consistent to line 517. ------------- PR: https://git.openjdk.org/jdk/pull/9781
