On Thu, 15 Jan 2026 08:56:21 GMT, Jatin Bhateja <[email protected]> wrote:
>> Add a new Float16lVector type and corresponding concrete vector classes, in >> addition to existing primitive vector types, maintaining operation parity >> with the FloatVector type. >> - Add necessary inline expander support. >> - Enable intrinsification for a few vector operations, namely >> ADD/SUB/MUL/DIV/MAX/MIN/FMA. >> - Use existing Float16 vector IR and backend support. >> - Extended the existing VectorAPI JTREG test suite for the newly added >> Float16Vector operations. >> >> The idea here is to first be at par with Float16 auto-vectorization support >> before intrinsifying new operations (conversions, reduction, etc). >> >> The following are the performance numbers for some of the selected >> Float16Vector benchmarking kernels compared to equivalent auto-vectorized >> Float16OperationsBenchmark kernels. >> >> <img width="1344" height="532" alt="image" >> src="https://github.com/user-attachments/assets/c8157c3c-22b0-4bc1-9de9-7a68cadb7b2a"; >> /> >> >> Initial RFP[1] was floated on the panama-dev mailing list. >> >> Kindly review the draft PR and share your feedback. >> >> Best Regards, >> Jatin >> >> [1] https://mail.openjdk.org/pipermail/panama-dev/2025-August/021100.html > > Jatin Bhateja has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 28 commits: > > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8370691 > - Adding testpoint for JDK-8373574 > - Review comments resolutions > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8370691 > - Fix incorrect argument passed to smokeTest > - Fix from Bhavana Kilambi for failing JTREG regressions on AARCH64 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8370691 > - Including test changes from Bhavana Kilambi (ARM) > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8370691 > - Optimizing tail handling > - ... and 18 more: https://git.openjdk.org/jdk/compare/499b5882...273b219e @jatin-bhateja I had a quick look at some of the changes. The patch is HUGE (80k+ lines), so it will take me a bit more time to review. I also realized that quite some changes are not directly related. For example, you are renaming lots of existing files. I would prefer if those changes were done separately. The issue is that at some point GitHub chokes, and it is no fun doing the review :/ src/hotspot/share/opto/vectorIntrinsics.cpp line 2895: > 2893: opd1 = gvn().transform(VectorNode::make(Op_AndV, opd1, > wrap_mask_vec, opd1->bottom_type()->is_vect())); > 2894: operation = > gvn().transform(VectorNode::make(Op_SelectFromTwoVector, opd1, opd2, opd3, > vt)); > 2895: VectorNode::trace_new_vector(operation, "VectorAPI"); I thought you wanted to add that in a separate RFE? src/hotspot/share/prims/vectorSupport.cpp line 206: > 204: } > 205: > 206: int VectorSupport::vop2ideal(jint id, int lane_type) { I think it would be nice if there was a name for `LaneType`. It's just nicer to have types named. After all, the code here used to use `BasicType`, and it helps the user know what is expected here. src/hotspot/share/prims/vectorSupport.cpp line 244: > 242: case T_FLOAT: return Op_MulF; > 243: case T_DOUBLE: return Op_MulD; > 244: default: fatal("MUL: %s", lanetype2name(lane_type)); You should fix the indentation here as well, since you are already doing it everywhere else ;) src/hotspot/share/utilities/globalDefinitions.hpp line 719: > 717: > 718: inline bool is_java_primitive(BasicType t) { > 719: return (t != T_FLOAT16 && T_BOOLEAN <= t && t <= T_LONG); This change seems unnecessary, right? `T_FLOAT16` is outside the range, as far as I can see. src/hotspot/share/utilities/globalDefinitions.hpp line 741: > 739: inline bool is_custom_basic_type(BasicType t) { > 740: return (t == T_FLOAT16); > 741: } What exactly is the definition of a "custom" basic type? Is it defined somewhere? If not, it would be useful to define it here. I assume you chose the name because we expect more such "custom" basic types in the future? ------------- Changes requested by epeter (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/28002#pullrequestreview-3678699226 PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r2705309070 PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r2705323981 PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r2705316085 PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r2705333596 PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r2705291064
