Unify processor detection on Z #3956

fjeremic · 2018-12-06T23:05:14Z

This PR contains several incremental changes. The end goal was to really print the machine id if the processor was unknown. Previously we extract the machine id and compare against a list of known ids. If no id matches we return 0, and when verbose printout is requested via -Xjit:verbose we print 0 as the machine id.

This is not ideal as we would like to print the machine id even if we did not match it to one of the known ones. We fix this problem here by eliminating the use of TR_S390MachineType enum and just use the machine id as an int32_t. In doing so we identify several other opportunities in the area of processor detection, such as severe code duplication, and responsibility movement to the port library.

Please see each commit for a description of what was changed and why.

fjeremic · 2018-12-06T23:05:27Z

Jenkins test sanity zlinux JDK8

runtime/compiler/z/env/J9CPU.cpp

fjeremic · 2018-12-06T23:08:13Z

runtime/compiler/z/env/J9CPU.cpp

-         {
-         ret_machine = (TR_S390MachineType)machine;
-         }
-      }


This is the logic that existed prior to this PR which would force us to return 0 as the machine id even though we did extract it.

fjeremic · 2018-12-06T23:09:25Z

runtime/compiler/z/env/J9CPU.cpp

@@ -415,215 +219,78 @@ CPU::getS390SupportsGuardedStorageFacility()
 ////////////////////////////////////////////////////////////////////////////////

 void
-CPU::initializeS390zLinuxProcessorFeatures()
+CPU::initializeS390ProcessorFeatures()


initializeS390zLinuxProcessorFeatures and initializeS390zOSProcessorFeatures are now unified into this function. I suggest to reviewers to compare the before and after for both functions against this new one and convince themselves that they are indeed the same semantically.

fjeremic · 2018-12-06T23:09:34Z

runtime/compiler/z/env/J9CPU.cpp

   {
   J9ProcessorDesc *processorDesc = TR::Compiler->target.cpu.TO_PORTLIB_getJ9ProcessorDesc();
   J9PortLibrary *privatePortLibrary = TR::Compiler->portLib;

   // The following conditionals are dependent on each other and must occur in this order
+   TR::Compiler->target.cpu.setS390SupportsZ9();


This was added as our ALS is z9.

fjeremic · 2018-12-06T23:14:28Z

I'll fix the copyright checks when I start addressing reviews so as to not cancel the currently running build.

joransiu

LGTM. Added a few questions / comments on things not directly related to the proposed changes.

runtime/compiler/env/ProcessorDetection.cpp

runtime/compiler/ras/DebugExt.cpp

runtime/compiler/z/env/J9CPU.cpp

runtime/port/unix/j9sysinfo.c

joransiu · 2018-12-16T04:55:56Z

I'll be out of office with questionable internet access. Perhaps @vijaysun-omr or @gita-omr would be able to take over this PR's review?

I think the only outstanding issue is related to HIGH_GPR support settings.

fjeremic · 2018-12-21T16:01:08Z

Unmarking WIP as everything has been addressed. @vijaysun-omr / @gita-omr are you able to take ownership of this PR as per the above comment by @joransiu?

gita-omr · 2018-12-21T16:14:58Z

Assigned to myself. Which tests should we run?

fjeremic · 2018-12-21T16:16:34Z

Assigned to myself. Which tests should we run?

This touches some common files as well. So to be conservative let's run everything:

Jenkins test sanity all JDK8

fjeremic · 2019-01-02T16:22:02Z

So it's been a while since this guy has been ready, however I will be away next week and back on the 13th. I would dislike if merging this change causes problems while I'm away and not here to fix anything. Having said that I think it's best to wait until my return before merging this.

I'll continue background testing to make sure nothing has regressed since.

Avoid using an enumerated `TR_S390MachineType` type to represent machine ids to future proof the JVM running on a more modern processor for which we do not know the machine id for. A typical example of this is running an older JVM, say very early Java 8 snapshots of OpenJ9 which did not have z14 support. In such cases the machine id for z14 would not match any of the existing machine ids in the codebase at the time and we would start reporting invalid machine ids in the verbose log ("0" to be precise). In this change we use the true machine id as parsed from z/OS or Linux on Z and display the number that is returned as an integer. The actual machine detection logic relies on the port library which uses STFLE bits to determine the processor we are executing on. This means processor detection is unaffected by our changes here. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

In the JIT compiler, similarly to j9sysinfo.c we can assume STFLE bits are always available since our ALS is z9. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

Both z/OS and Linux should rely on the answers given by the port library on whether we support particular processor features. The code was already more or less the same between the two functions, however there were some redundancies in checking for OS support as the port library already takes care of this for us in j9sysinfo.c Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

Several of these functions are already implemented in j9sysinfo.c where they really belong. Deprecate the support in the JIT. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

We cannot use `utsname` on Linux as it simply returns "s390x" in `info.machine`. Add a comment directly in the code as to why this restriction is there. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

fjeremic · 2019-01-17T17:12:06Z

@joransiu I have removed the "enabling HPRs on Linux on Z 31-bit" commit from this PR. The reason is because I have another rather large change coming to remove TR_GPR64 from the codebase and consolidate handling of 64-bit registers across all Z platforms. Because this is a massive RA change (~2000 lines or so) I don't feel comfortable making this HPR change beforehand. I'd rather bundle it as part of the RA change to make sure nothing gets broken.

This PR should be ready to go. Launching builds on your behalf.

Jenkins test sanity all JDK8

joransiu

Reviewed the commits again. LGTM.

fjeremic added the comp:jit label Dec 6, 2018

fjeremic assigned joransiu Dec 6, 2018

fjeremic requested a review from joransiu December 6, 2018 23:05

fjeremic commented Dec 6, 2018

View reviewed changes

joransiu approved these changes Dec 10, 2018

View reviewed changes

runtime/compiler/env/ProcessorDetection.cpp Show resolved Hide resolved

runtime/compiler/env/ProcessorDetection.cpp Show resolved Hide resolved

runtime/compiler/ras/DebugExt.cpp Show resolved Hide resolved

runtime/compiler/z/env/J9CPU.cpp Show resolved Hide resolved

fjeremic force-pushed the fix-processor-detection-z branch from 1cab02a to 332b5f3 Compare December 10, 2018 19:06

fjeremic commented Dec 10, 2018

View reviewed changes

runtime/port/unix/j9sysinfo.c Outdated Show resolved Hide resolved

fjeremic changed the title ~~Unify processor detection on Z~~ WIP: Unify processor detection on Z Dec 10, 2018

fjeremic force-pushed the fix-processor-detection-z branch from 41df2f3 to 4522048 Compare December 17, 2018 20:54

fjeremic changed the title ~~WIP: Unify processor detection on Z~~ Unify processor detection on Z Dec 21, 2018

gita-omr self-assigned this Dec 21, 2018

fjeremic added 4 commits January 17, 2019 12:04

Assume STFLE bits are always available since ALS is z9

7fe6127

In the JIT compiler, similarly to j9sysinfo.c we can assume STFLE bits are always available since our ALS is z9. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

Deprecate several unused functions in Z J9CPU

221ea79

Several of these functions are already implemented in j9sysinfo.c where they really belong. Deprecate the support in the JIT. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

fjeremic force-pushed the fix-processor-detection-z branch from 4522048 to 9ae9991 Compare January 17, 2019 17:04

Add comment regarding utsname restriction on Linux on Z

fd8342d

We cannot use `utsname` on Linux as it simply returns "s390x" in `info.machine`. Add a comment directly in the code as to why this restriction is there. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>

fjeremic force-pushed the fix-processor-detection-z branch from 9ae9991 to fd8342d Compare January 17, 2019 17:08

joransiu approved these changes Jan 17, 2019

View reviewed changes

joransiu merged commit 1c77628 into eclipse-openj9:master Jan 17, 2019

fjeremic deleted the fix-processor-detection-z branch January 17, 2019 20:25

fjeremic mentioned this pull request Apr 24, 2019

Add processor version in javacore file #5532

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify processor detection on Z #3956

Unify processor detection on Z #3956

fjeremic commented Dec 6, 2018

fjeremic commented Dec 6, 2018

fjeremic Dec 6, 2018

fjeremic Dec 6, 2018

fjeremic Dec 6, 2018

fjeremic commented Dec 6, 2018

joransiu left a comment

joransiu commented Dec 16, 2018

fjeremic commented Dec 21, 2018

gita-omr commented Dec 21, 2018

fjeremic commented Dec 21, 2018

fjeremic commented Jan 2, 2019

fjeremic commented Jan 17, 2019

joransiu left a comment

Unify processor detection on Z #3956

Unify processor detection on Z #3956

Conversation

fjeremic commented Dec 6, 2018

fjeremic commented Dec 6, 2018

fjeremic Dec 6, 2018

Choose a reason for hiding this comment

fjeremic Dec 6, 2018

Choose a reason for hiding this comment

fjeremic Dec 6, 2018

Choose a reason for hiding this comment

fjeremic commented Dec 6, 2018

joransiu left a comment

Choose a reason for hiding this comment

joransiu commented Dec 16, 2018

fjeremic commented Dec 21, 2018

gita-omr commented Dec 21, 2018

fjeremic commented Dec 21, 2018

fjeremic commented Jan 2, 2019

fjeremic commented Jan 17, 2019

joransiu left a comment

Choose a reason for hiding this comment