RFR: 8339220: [s390x] TestIfMinMax.java failure [v2]

[Amit Kumar]

On Thu, 26 Sep 2024 10:35:42 GMT, Lutz Schmidt <lucy at openjdk.org> wrote:

>> Amit Kumar has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   suggestions from Andrew
>
> src/hotspot/cpu/s390/matcher_s390.hpp line 69:
> 
>> 67:   static int long_cmove_cost() {
>> 68:     // z196/z11 or later hardware support conditional moves
>> 69:     return VM_Version::get_model_index() >= 5 ? 0 : ConditionalMoveLimit;
> 
> Why didn't you use `has_LoadStoreConditional()`? This method uses the facility indication flag and does not rely on some artificial architecture generation detection. At least in theory, it could happen that the facility you are testing for was not installed or disabled on the machine you are running on.

Fixed :)

> src/hotspot/cpu/x86/vm_version_x86.hpp line 656:
> 
>> 654:   //       4   -  486
>> 655:   //       5   -  Pentium
>> 656:   //       6   -  PentiumPro, Pentium II, Celer;on, Xeon, Pentium III, Athlon,
> 
> I haven't heard of that processor family. Nor does intel talk about it: https://www.intel.com/content/www/us/en/products/details/processors/celeron.html

Oh, maybe because it's not released yet. 

sorry, that was unintentional-typo; Fixed :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21198#discussion_r1777020322
PR Review Comment: https://git.openjdk.org/jdk/pull/21198#discussion_r1777023337