On Mon, 8 Jun 2026 07:16:17 GMT, Xiaohong Gong <[email protected]> wrote:
>> Thanks for the explanations! Sounds like we now might have a bit of a design >> mess with float16/short confusion in the VM. I hope we can get to a clean >> design in the future. >> >> But about `is_unsupported_lane_type` specifically: my concern is that it is >> not good design. Now you might argue it is transitional code only, and so we >> should be ok with bad design here. But it often happens that transitional >> code stays for a while, and it would be quite easy to flip this to better >> design. >> >> My opinion: >> **Don't exclude bad things. Only allow good things.** >> Otherwise, you'll at some point forget another bad thing, and forget to >> exclude it, and we get incorrect code. >> That's worse than forgetting a good thing: we just get slower but still >> correct code. >> >> So I'd ask you to flip it to a `is_supported_lane_type`, and then just >> accept only primitive types for now, and later also allow float16, once we >> make those extensions. > > I see, thanks! I was thinking there is a new BasicType for FP16 added as > well. I do not suggest adding new IRs for FP16 types. Do you think it's > better to maintain a list of supported vector ops for FP16 instead of > checking the type for each intrinsic? We can check the list firstly in > `arch_supports_vector`, and remove the list once all the vector ops are > supported in future. Hi @XiaohongGong , Looking at the interface of [arch_supported_vector](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/vectorIntrinsics.cpp#L193), it again accepts vector opcode , element type and num_elems etc... so we will need an additional argument which demarcates Float16 vector operations based on java side LaneType and then comparing it against a supported vector list will be cumbersome and still need changes in each inline expander. Hi @eme64, Good point — I've flipped is_unsupported_lane_type into an allowlist is_supported_lane_type that accepts only the primitive lane types for now. I will remove these calls one by one from each inline expander as we add the support for Float16Vector operations incrementally. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r3371746707
