On Mon, 8 Jun 2026 04:44:14 GMT, Jatin Bhateja <[email protected]> wrote:
>> I left a similar comment above >> https://github.com/openjdk/jdk/pull/28002#discussion_r3115506353. I think it >> would be better if we could handle such unsupported types in >> `match_rule_supported_vector` or `arch_supports_vector`. Although that might >> need changes on all vector supported backends, it deserves the effort if >> that is the right direction. > > Hi @XiaohongGong , > > Take inline_vector_compare as an example. The lane type passed from the Java > side is LT_FLOAT16, but match_rule_supported_vector operates on the vector IR > opcode, the basic type (which for Float16 is short), and num_elem. So a > Float16 compare would be matched as Op_VectorMaskCmp with elem_bt=short and > collide with the existing short-integer VectorMaskCmpNode used by ShortVector > — giving wrong FP semantics. > > We would need a distinct Float16 compare IR (as we already did for the other > supported vector Float16 ops) before this can be guarded purely by > match_rule_supported_vector. That is why, for now, suppressing > intrinsification based on lane type is the clean approach; in subsequent > patches we can drop the is_unsupported_lane_type checks one by one and rely > on match_rule_supported_vector with the new opcodes, which will also guard > platforms lacking support. > > This PR mainly adds the Java-side support and enables intrinsification only > for operations that already have auto-vectorization support. I hope this > looks reasonable. 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r3371353644
