On Tue, 11 Nov 2025 16:28:54 GMT, Paul Sandoz <[email protected]> wrote:
>> We already have a lot of things in the codebase now from previous issues >> that use `HF` everywhere, for example some node names, and the type. Should >> we maybe rename all of them to `F16`, or something else? Open question, not >> sure of the answer yet. > >> We already have a lot of things in the codebase now from previous issues >> that use `HF` everywhere, for example some node names, and the type. Should >> we maybe rename all of them to `F16`, or something else? Open question, not >> sure of the answer yet. > > I was only referring to the Java code, esp. the new public classes so they > align with the `Float16` element type. I do think it worthwhile to align so > we are consistent across the platform. Revisiting the names in HotSpot, and > their internal connection in Java, could be done in a separate PR? Hi @PaulSandoz , Thanks for your comments. Please find below my responses. > When you generate the fallback code for unary/binary etc can you push the > carrier type and conversations into the uOp/bOp implementations so you don't > have to explicitly operate on the carrier type and do the conversions as you > do now e.g.,: > > ``` > v0.uOp(m, (i, a) -> > float16ToShortBits(Float16.valueOf(-(shortBitsToFloat16(($type$)a).floatValue())))); > ``` Currently, uOp and uOpTemplates are part of the scaffolding logic and are sacrosanct; they are shared by various abstracted vector classes, and their semantics are defined by the lambda expression. I agree that explicit conversion in lambdas looks verbose, but moving them to uOpTemplate may fracture the lambda expression such that part of its semantics, i.e,. conversions, will seep into uOpTemplate, while what will appear at the surface will be the expression operating over primitive float values; this may become very confusing. > > The transition of intrinsic arguments from `vsp.elementType()` to > `vsp.carrierType(), vsp.operType()` is a little unfortunate. Is this because > HotSpot cannot directly refer to the `Float16` class from the incubating > module? Yes, the idea here was to clearly differentiate b/w elemType and carrierType and avoid passing Float16.class as an argument to intrinsic entry points. Unlike the VectorSupport class, Float16 is part of the incubating module and cannot be directly exposed to VM, i.e., we cannot create a vmSymbol for it during initialization. This would have made all the lane type checks in-line expand name-based rather than efficient symbol lookup. > Requiring two arguments means they can get out of sync. Previously the class > provided all the information needed, now > arguably the type does. Yes, from the compiler standpoint point all we care about is the carrier type, which determines the vector lane size. This is augmented with operation kind (PRIM / FP16) to differentiate a short vector lane from a float16 vector lane. Apart from this, we need to pass the VectorBox type to wrap the vector IR. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28002#issuecomment-3520530639
