RFC [GR-52835] Support OpenJDK versions in JVMCIVersionCheck

PullRequest: graal/17419

Author: zapster

compiler/mx.compiler/mx_compiler.py

@@ -25,6 +25,7 @@
 
 from __future__ import print_function
 import os
+from functools import total_ordering
 from os.path import join, exists, basename, dirname, isdir
 import argparse
 from argparse import ArgumentParser, RawDescriptionHelpFormatter, REMAINDER
@@ -79,7 +80,7 @@ def get_vm_prefix(asList=True):
 
 
 class JavaLangRuntimeVersion(mx.Comparable):
-    """Wrapper for by java.lang.Runtime.Version"""
+    """Wrapper for java.lang.Runtime.Version"""
 
     _cmp_cache = {}
     _feature_re = re.compile('[1-9][0-9]*')
@@ -124,10 +125,48 @@ def feature(self):
             self._feature = int(JavaLangRuntimeVersion._feature_re.match(self.version).group(0))
         return self._feature
 
+@total_ordering
+class JVMCIVersionCheckVersion(object):
+    def __init__(self, jdk_version, jvmci_major, jvmci_minor, jvmci_build):
+        """
+        Python version of jdk.graal.compiler.hotspot.JVMCIVersionCheck.Version
+
+        jdk_version is a JavaLangRuntimeVersion
+        jvmci_major and jvmci_minor might be 0 if not needed (JDK 22+)
+        """
+        assert isinstance(jdk_version, JavaLangRuntimeVersion)
+        assert isinstance(jvmci_major, int)
+        assert isinstance(jvmci_minor, int)
+        assert isinstance(jvmci_build, int)
+        self.jdk_version = jdk_version
+        self.jvmci_major = jvmci_major
+        self.jvmci_minor = jvmci_minor
+        self.jvmci_build = jvmci_build
+
+    def _as_tuple(self):
+        return (self.jdk_version, self.jvmci_major, self.jvmci_minor, self.jvmci_build)
+
+    def __eq__(self, other):
+        if not isinstance(other, JVMCIVersionCheckVersion):
+            return False
+        return self._as_tuple() == other._as_tuple()
+
+    def __lt__(self, other):
+        if not isinstance(other, JVMCIVersionCheckVersion):
+            return NotImplemented
+        return self._as_tuple() < other._as_tuple()
+
+    def __str__(self):
+        jdk_version, jvmci_major, jvmci_minor, jvmci_build = self._as_tuple()
+        if jvmci_major == 0:
+            if jvmci_build == 0:
+                return f'(openjdk|oraclejdk)-{jdk_version}'
+            else:
+                return f'labsjdk-(ce|ee)-{jdk_version}-jvmci-b{jvmci_build:02d}'
+        else:
+            return f'labsjdk-(ce|ee)-{jdk_version}-jvmci-{jvmci_major}.{jvmci_minor}-b{jvmci_build:02d}'
+
 
-#: 4-tuple (jdk_version, jvmci_major, jvmci_minor, jvmci_build) of JVMCI version, if any, denoted by `jdk`
-# jdk_version is a JavaLangRuntimeVersion
-# jvmci_major and jvmci_minor might be 0 if not needed (JDK 22+)
 _jdk_jvmci_version = None
 _jdk_min_jvmci_version = None
 
@@ -145,7 +184,7 @@ def _capture_jvmci_version(args=None):
         if out.data:
             try:
                 (jdk_version, jvmci_major, jvmci_minor, jvmci_build) = out.data.split(',')
-                return (JavaLangRuntimeVersion(jdk_version), int(jvmci_major), int(jvmci_minor), int(jvmci_build))
+                return JVMCIVersionCheckVersion(JavaLangRuntimeVersion(jdk_version), int(jvmci_major), int(jvmci_minor), int(jvmci_build))
             except ValueError:
                 mx.warn(f'Could not parse jvmci version from JVMCIVersionCheck output:\n{out.data}')
             return None
@@ -1165,7 +1204,7 @@ def _check_latest_jvmci_version():
     ``common.json`` file and issues a warning if not.
     """
     jvmci_re = re.compile(r'(?:ce|ee)-(?P<jdk_version>.+)-jvmci(?:-(?P<jvmci_major>\d+)\.(?P<jvmci_minor>\d+))?-b(?P<jvmci_build>\d+)')
-    common_path = join(_suite.dir, '..', 'common.json')
+    common_path = os.path.normpath(join(_suite.dir, '..', 'common.json'))
 
     if _jdk_jvmci_version is None:
         # Not using a JVMCI JDK
@@ -1183,8 +1222,13 @@ def get_latest_jvmci_version():
                 if not match:
                     mx.abort(f'Cannot parse version {version}')
                 (jdk_version, jvmci_major, jvmci_minor, jvmci_build) = match.groups(default=0)
-                current = (JavaLangRuntimeVersion(jdk_version), int(jvmci_major), int(jvmci_minor), int(jvmci_build))
-                if current[0].feature() == _jdk_jvmci_version[0].feature():
+                if _jdk_jvmci_version.jvmci_build == 0:
+                    # jvmci_build == 0 indicates an OpenJDK version has been specified in JVMCIVersionCheck.java.
+                    # The JDK does not know the jvmci_build number that might have been specified in common.json,
+                    # as it is only a repackaged JDK. Thus, we reset the jvmci_build because we cannot validate it.
+                    jvmci_build = 0
+                current = JVMCIVersionCheckVersion(JavaLangRuntimeVersion(jdk_version), int(jvmci_major), int(jvmci_minor), int(jvmci_build))
+                if current.jdk_version.feature() == _jdk_jvmci_version.jdk_version.feature():
                     # only compare the same major versions
                     if latest == 'not found':
                         latest = current
@@ -1196,28 +1240,21 @@ def get_latest_jvmci_version():
                         return False, distribution
         return not isinstance(latest, str), latest
 
-    def jvmci_version_str(version):
-        jdk_version, jvmci_major, jvmci_minor, jvmci_build = version
-        if jvmci_major == 0:
-            return f'labsjdk-(ce|ee)-{jdk_version}-jvmci-b{jvmci_build:02d}'
-        else:
-            return f'labsjdk-(ce|ee)-{jdk_version}-jvmci-{jvmci_major}.{jvmci_minor}-b{jvmci_build:02d}'
-
     version_check_setting = os.environ.get('JVMCI_VERSION_CHECK', None)
 
     success, latest = get_latest_jvmci_version()
 
     if version_check_setting == 'strict' and _jdk_jvmci_version != _jdk_min_jvmci_version:
         msg = f'JVMCI_MIN_VERSION specified in JVMCIVersionCheck.java is older than in {common_path}:'
-        msg += os.linesep + f'{jvmci_version_str(_jdk_min_jvmci_version)} < {jvmci_version_str(_jdk_jvmci_version)} '
+        msg += os.linesep + f'{_jdk_min_jvmci_version} < {_jdk_jvmci_version} '
         msg += os.linesep + f'Did you forget to update JVMCI_MIN_VERSION after updating {common_path}?'
         msg += os.linesep + 'Set the JVMCI_VERSION_CHECK environment variable to something else then "strict" to'
         msg += ' suppress this error.'
         mx.abort(msg)
 
     if version_check_setting == 'strict' and not success:
         if latest == 'not found':
-            msg = f'No JVMCI JDK found in {common_path} that matches {jvmci_version_str(_jdk_jvmci_version)}.'
+            msg = f'No JVMCI JDK found in {common_path} that matches {_jdk_jvmci_version}.'
             msg += os.linesep + f'Check that {latest} matches the versions of the other JVMCI JDKs.'
         else:
             msg = f'Version mismatch in {common_path}:'
@@ -1227,10 +1264,9 @@ def jvmci_version_str(version):
         mx.abort(msg)
 
     if success and _jdk_jvmci_version < latest:
-        common_path = os.path.normpath(common_path)
-        msg = f'JVMCI version of JAVA_HOME is older than in {common_path}: {jvmci_version_str(_jdk_jvmci_version)} < {jvmci_version_str(latest)} '
+        msg = f'JVMCI version of JAVA_HOME is older than in {common_path}: {_jdk_jvmci_version} < {latest} '
         msg += os.linesep + 'This poses the risk of hitting JVMCI bugs that have already been fixed.'
-        msg += os.linesep + f'Consider using {jvmci_version_str(latest)}, which you can get via:'
+        msg += os.linesep + f'Consider using {latest}, which you can get via:'
         msg += os.linesep + f'mx fetch-jdk --configuration {common_path}'
         mx.abort_or_warn(msg, version_check_setting == 'strict')
 

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/JVMCIVersionCheckMaxValueTest.java

@@ -54,7 +54,7 @@ public void testNewVersion() {
 
     private static void testVersion(String javaVmVersion) {
         try {
-            JVMCIVersionCheck.Version minVersion = new JVMCIVersionCheck.Version(20, 0, 1);
+            JVMCIVersionCheck.Version minVersion = JVMCIVersionCheck.createLegacyVersion(20, 0, 1);
             // Use a javaSpecVersion that will likely not fail in the near future
             String javaSpecVersion = "99";
             var props = JVMCIVersionCheckTest.createTestProperties(javaSpecVersion, javaVmVersion, null);

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/JVMCIVersionCheckOpenJDKTest.java

@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.graal.compiler.hotspot.test;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import jdk.graal.compiler.core.test.GraalCompilerTest;
+import jdk.graal.compiler.hotspot.JVMCIVersionCheck;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import static jdk.graal.compiler.hotspot.JVMCIVersionCheck.DEFAULT_VENDOR_ENTRY;
+
+/**
+ * Tests that {@link JVMCIVersionCheck} handles OpenJDK versions correctly.
+ */
+@RunWith(Parameterized.class)
+public class JVMCIVersionCheckOpenJDKTest extends GraalCompilerTest {
+
+    @Parameterized.Parameters(name = "{0} <= {1} = {2}")
+    public static Collection<Object[]> data() {
+        return List.of(
+                        /*
+                         * If comparing a LabsJDK version against an OpenJDK version, ignore the
+                         * JVMCI build number.
+                         */
+                        expectFail("99+99-jvmci-b02", "99+98"),
+                        expectPass("99+99-jvmci-b02", "99+99"),
+                        expectPass("99+99-jvmci-b02", "99+100"),
+                        /*
+                         * If comparing a LabsJDK version against a LabsJDK version, respect the
+                         * JVMCI build.
+                         */
+                        expectFail("99+99-jvmci-b02", "99+98-jvmci-b01"),
+                        expectFail("99+99-jvmci-b02", "99+98-jvmci-b02"),
+                        expectFail("99+99-jvmci-b02", "99+98-jvmci-b03"),
+
+                        expectFail("99+99-jvmci-b02", "99+99-jvmci-b01"),
+                        expectPass("99+99-jvmci-b02", "99+99-jvmci-b02"),
+                        expectPass("99+99-jvmci-b02", "99+99-jvmci-b03"),
+
+                        expectPass("99+99-jvmci-b02", "99+100-jvmci-b01"),
+                        expectPass("99+99-jvmci-b02", "99+100-jvmci-b02"),
+                        expectPass("99+99-jvmci-b02", "99+100-jvmci-b03"),
+
+                        /* Comparing an OpenJDK version against an OpenJDK version. */
+                        expectFail("99+99", "99+98"),
+                        expectPass("99+99", "99+99"),
+                        expectPass("99+99", "99+100"),
+
+                        /* Comparing an OpenJDK version against a LabsJDK version. */
+                        expectFail("99+99", "99+98-jvmci-b01"),
+                        expectPass("99+99", "99+99-jvmci-b01"),
+                        expectPass("99+99", "99+100-jvmci-b01"));
+    }
+
+    private static Object[] expectPass(String minVersion, String javaVmVersion) {
+        return new Object[]{minVersion, javaVmVersion, true};
+    }
+
+    private static Object[] expectFail(String minVersion, String javaVmVersion) {
+        return new Object[]{minVersion, javaVmVersion, false};
+    }
+
+    @Parameterized.Parameter(0) public String minVersion;
+    @Parameterized.Parameter(1) public String javaVmVersion;
+    @Parameterized.Parameter(2) public boolean expectSuccess;
+
+    @Test
+    public void compareToMinVersion() {
+        if (checkVersionProperties(getMinVersionMap(minVersion), javaVmVersion)) {
+            if (!expectSuccess) {
+                Assert.fail(String.format("Expected %s to be older than %s", javaVmVersion, minVersion));
+            }
+        } else {
+            if (expectSuccess) {
+                Assert.fail(String.format("Expected %s not to be older than %s", javaVmVersion, minVersion));
+            }
+
+        }
+    }
+
+    private static Map<String, Map<String, JVMCIVersionCheck.Version>> getMinVersionMap(String minVersion) {
+        Runtime.Version version = Runtime.Version.parse(minVersion);
+        if (version.optional().isEmpty()) {
+            // OpenJDK version
+            return Map.of(Integer.toString(version.feature()), Map.of(
+                            DEFAULT_VENDOR_ENTRY, JVMCIVersionCheck.createOpenJDKVersion(minVersion)));
+        } else {
+            // LabsJDK version
+            String optional = version.optional().get();
+            // get the jvmci build number
+            Assert.assertTrue("expected jvmci build number", optional.startsWith("jvmci-b"));
+            int jvmciBuild = Integer.parseInt(optional.split("jvmci-b", 2)[1]);
+            // get the version string without the option part
+            String versionWithoutOptional = version.toString().split("-" + optional, 2)[0];
+            return Map.of(Integer.toString(version.feature()), Map.of(
+                            DEFAULT_VENDOR_ENTRY, JVMCIVersionCheck.createLabsJDKVersion(versionWithoutOptional, jvmciBuild)));
+        }
+    }
+
+    private static boolean checkVersionProperties(Map<String, Map<String, JVMCIVersionCheck.Version>> minVersionMap, String javaVmVersion) {
+        String javaSpecVersion = Integer.toString(Runtime.Version.parse(javaVmVersion).feature());
+        var props = JVMCIVersionCheckTest.createTestProperties(javaSpecVersion, javaVmVersion, null);
+        try {
+            JVMCIVersionCheck.check(props, false, null, minVersionMap);
+            return true;
+        } catch (InternalError e) {
+            return false;
+        }
+    }
+
+}

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/JVMCIVersionCheckTest.java

@@ -110,10 +110,10 @@ public static Collection<Object[]> data() {
     private static Version getVersion(String jdkVersion, int major, int minor, int build) {
         if (jdkVersion != null) {
             // new version scheme
-            return new Version(jdkVersion, build);
+            return JVMCIVersionCheck.createLabsJDKVersion(jdkVersion, build);
         } else {
             // legacy version scheme
-            return new Version(major, minor, build);
+            return JVMCIVersionCheck.createLegacyVersion(major, minor, build);
         }
     }
 

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/JVMCIVersionCheckVendorTest.java

@@ -41,8 +41,8 @@
 public class JVMCIVersionCheckVendorTest extends GraalCompilerTest {
 
     private static final Map<String, Map<String, JVMCIVersionCheck.Version>> VERSION_MAP = Map.of("99", Map.of(
-                    DEFAULT_VENDOR_ENTRY, new JVMCIVersionCheck.Version("99+99", 1),
-                    "Vendor Specific", new JVMCIVersionCheck.Version("99.0.1", 1)));
+                    DEFAULT_VENDOR_ENTRY, JVMCIVersionCheck.createLabsJDKVersion("99+99", 1),
+                    "Vendor Specific", JVMCIVersionCheck.createLabsJDKVersion("99.0.1", 1)));
 
     private static void expect(String javaVmVendor, String expected) {
         var props = JVMCIVersionCheckTest.createTestProperties("99", null, javaVmVendor);