On Fri, 23 Jan 2026 04:57:04 GMT, Jatin Bhateja <[email protected]> wrote:

>> Hi @eme64 , Your comments have been addressed
>
>> @jatin-bhateja This patch is really really large. There are lots of 
>> renamings that could be done in a separate patch first (as a subtask). It 
>> would make reviewing easier, allowing focus on the substantial work. See 
>> discussion here: [#28002 
>> (comment)](https://github.com/openjdk/jdk/pull/28002#discussion_r2705376899)
> 
> Hi @eme64 ,
> 
> I have done some cleanups, following is the summary of changes included with 
> the patch:-
> 
>  ```
>  1 Changes to introduce a new (custom) basictype T_FLOAT16
>         - Global Definition.
>         - Skip over handling where ever applicable.
>   2 Changes to pass laneType (BasicType) to intrinsific entry point instead 
> of element classes.
>        - Inline expander interface changes mainly.
>   3 Changes in abstract and concrete vector class generation templates.
>   4 Changing the nomenclature of Vector classes to avoid Float1664... sort of 
> names...
>   5 Changes in the LaneType to add a new carrier type field.    
>   6 Changes in inline expanders to selectivelty enable intrinsification for 
> opration for which we have
>     auto-vectorization and backend support in place..
>   7 Changes in test generation templates.
>        b. Assert wrappers to conver float16 (short) value to float before 
> invoking testng Asserts.
>        c. Scalar operation wrappers to selectivelty invoke Float16 math 
> routine which are not 
>          part of Java SE math libraries.
>    
>   8 New IR verification test.
>   9 New Micro-benchmark.
>   10 AARCH64 test failure - patch + test fixed by Bhavana Kilambi.  
> 
> 
> Out of above change 7b consumes 40000+ LOC.
> 
> Q. Why do we need wrapper assertions ?
> A.  To handle all possible NaN representations of SNaN and QNaN, since 
> float16 uses short carrier type hence we need to promote them float values 
> before invoking TestNG assertions. This conversion is accomplished by 
> assertion wrappers
>  
> All the tasks are related and most of source/test are generated using scripts 
> we should not go by the size of patch and review the templates files.

@jatin-bhateja I was wondering: what prompted the decision to add a new 
`BasicType` for `Float16`? Would each additional numeric type get a new 
`BasicType`? How many do we anticipate?

Currently, we are using `T_SHORT` for `Float16`, right?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/28002#issuecomment-3800362594

Reply via email to