Re: RFR: 8342103: C2 compiler support for Float16 type and associated operations [v3]

[Emanuel Peter]

On Tue, 26 Nov 2024 08:25:46 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolution
>
>> Another example where I asked if we have good tests: 
>> ![image](https://private-user-images.githubusercontent.com/32593061/389841818-8fafd51e-9fed-453f-aedb-7dc6d6d17cc1.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzI2MDg3MDMsIm5iZiI6MTczMjYwODQwMywicGF0aCI6Ii8zMjU5MzA2MS8zODk4NDE4MTgtOGZhZmQ1MWUtOWZlZC00NTNmLWFlZGItN2RjNmQ2ZDE3Y2MxLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMjYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTI2VDA4MDY0M1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTMwZTBhOTVjOGRmNzViY2ZjYWU0M2E3ZmE1ZWEzYmYzY2E1YmQxN2JiZDkwOGJiYjZhNTcxZTFmZDc3MGU2ZjEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.-qd93PHlVMGcEbMblqKRIgdGc6tj-M7sq4oglGpgtSA)
>> 
>> And the test you point to is this: 
>> ![image](https://private-user-images.githubusercontent.com/32593061/389841921-0bfda1d7-7bc0-4e5b-8ea7-171a02a805ff.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzI2MDg3MDMsIm5iZiI6MTczMjYwODQwMywicGF0aCI6Ii8zMjU5MzA2MS8zODk4NDE5MjEtMGJmZGExZDctN2JjMC00ZTViLThlYTctMTcxYTAyYTgwNWZmLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMjYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTI2VDA4MDY0M1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWJiMWIzYWUzYjY0NDE0NWUzMzYwMTAxMDk3MzM2YmU1MzdhNjlhZjk0ODdjN2U4OTZjMmI5YWVlMTZmMDkwZjEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.bpkhyUSEqf80pl8reM1Wa7OCvPX6Z3muzqlWOVMCnjs)
>> 
>> It only covers a single constant `divisor = 8`. But what about divisors that 
>> are out of the allowed range, or not powers of 2? How do we know that you 
>> chose the bounds correctly, and are not off-by-1? And what about negative 
>> divisors? 
>> ![image](https://private-user-images.githubusercontent.com/32593061/389842530-8f2260e5-0075-4d34-9d30-2cec817c72f2.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzI2MDg3MDMsIm5iZiI6MTczMjYwODQwMywicGF0aCI6Ii8zMjU5MzA2MS8zODk4NDI1MzAtOGYyMjYwZTUtMDA3NS00ZDM0LTlkMzAtMmNlYzgxN2M3MmYyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMjYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTI2VDA4MDY0M1omWC1BbXotRXh...

@jatin-bhateja 
> I can feel the reviewer's pain

Then please do something about it!

Your comments are helpful. But they do not answer my request for better test 
coverage.

Yes, `gtest` would be helpful.
But also Java end-to-end tests are required.

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

PR Comment: https://git.openjdk.org/jdk/pull/21490#issuecomment-2499977879