gcc10 warnings in the JIT

[pshipton]

See the nightly builds at https://openj9-jenkins.osuosl.org/job/Pipeline-Compile-gcc10-JDK17/

17:48:53  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/compile/OSRData.cpp: In member function 'void TR_OSRCompilationData::addInstruction(int32_t, TR_ByteCodeInfo)':
17:48:53  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/compile/OSRData.cpp:117:1: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
17:48:53    117 | TR_OSRCompilationData::addInstruction(int32_t instructionPC, TR_ByteCodeInfo bcInfo)
17:48:53        | ^~~~~~~~~~~~~~~~~~~~~

...

17:49:02  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp: In member function 'TR::TreeTop* OMR::ResolvedMethodSymbol::genInduceOSRCallAndCleanUpFollowingTreesImmediately(TR::TreeTop*, TR_ByteCodeInfo, bool, TR::Compilation*)':
17:49:02  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp:585:1: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
17:49:02    585 | OMR::ResolvedMethodSymbol::genInduceOSRCallAndCleanUpFollowingTreesImmediately(TR::TreeTop *insertionPoint, TR_ByteCodeInfo induceBCI, bool shouldSplitBlock, TR::Compilation *comp)
17:49:02        | ^~~
17:49:02  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp: In member function 'TR::TreeTop* OMR::ResolvedMethodSymbol::induceOSRAfterImpl(TR::TreeTop*, TR_ByteCodeInfo, TR::TreeTop*, bool, int32_t, TR::TreeTop**)':
17:49:02  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp:516:1: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
17:49:02    516 | OMR::ResolvedMethodSymbol::induceOSRAfterImpl(TR::TreeTop *insertionPoint, TR_ByteCodeInfo induceBCI, TR::TreeTop* branch,
17:49:02        | ^~~
17:49:03  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp: In member function 'bool OMR::ResolvedMethodSymbol::induceOSRAfterAndRecompile(TR::TreeTop*, TR_ByteCodeInfo, TR::TreeTop*, bool, int32_t, TR::TreeTop**)':
17:49:03  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp:480:1: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
17:49:03    480 | OMR::ResolvedMethodSymbol::induceOSRAfterAndRecompile(TR::TreeTop *insertionPoint, TR_ByteCodeInfo induceBCI, TR::TreeTop* branch,
17:49:03        | ^~~
17:49:03  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp: In member function 'bool OMR::ResolvedMethodSymbol::induceOSRAfter(TR::TreeTop*, TR_ByteCodeInfo, TR::TreeTop*, bool, int32_t, TR::TreeTop**)':
17:49:03  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/il/OMRResolvedMethodSymbol.cpp:503:1: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
17:49:03    503 | OMR::ResolvedMethodSymbol::induceOSRAfter(TR::TreeTop *insertionPoint, TR_ByteCodeInfo induceBCI, TR::TreeTop* branch,
17:49:03        | ^~~

...

17:49:48  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/optimizer/OSRDefAnalysis.cpp: In member function 'TR::TreeTop* TR_OSRLiveRangeAnalysis::collectPendingPush(TR_ByteCodeInfo, TR::TreeTop*, TR_BitVector*)':
17:49:48  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/optimizer/OSRDefAnalysis.cpp:1559:14: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
17:49:48   1559 | TR::TreeTop *TR_OSRLiveRangeAnalysis::collectPendingPush(TR_ByteCodeInfo bci, TR::TreeTop *tt, TR_BitVector *liveVars)
17:49:48        |              ^~~~~~~~~~~~~~~~~~~~~~~

Also this

17:50:25  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/aarch64/codegen/OMRTreeEvaluator.cpp: In function 'void addMetaDataForLoadAddressConstantFixed(TR::CodeGenerator*, TR::Node*, TR::Instruction*, int16_t, intptr_t)':
17:50:25  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/aarch64/codegen/OMRTreeEvaluator.cpp:2497:13: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
17:50:25   2497 |             (uint8_t *)node->getInlinedSiteIndex(),
17:50:25        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17:50:25  /home/jenkins/workspace/Build_JDK17_aarch64_linux_gcc10_Personal/omr/compiler/aarch64/codegen/OMRTreeEvaluator.cpp:2531:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
17:50:25   2531 |                (uint8_t *)(node == NULL ? -1 : node->getInlinedSiteIndex()),
17:50:25        |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@0xdaryl @knn-k

[0xdaryl]

TR_ByteCodeInfo is a bitfield, and we are likely running into some variant of this problem: https://bugs.openjdk.java.net/browse/JDK-8235983

[knn-k]

I fixed AArch64-specific warnings with eclipse/omr#6461 and #14876.

[vij-singh]

@0xdaryl Is there anything else left to do on this one?

[0xdaryl]

Yes, we can suppress the gcc ABI warning on AArch64. It is generated because of a gcc bug fix implementing the AArch64 ABI for bitfields, and may appear if mixing object files built with different compiler releases. We don't do that in OpenJ9 so we can suppress the warning. OpenJDK has also suppressed the warning.

I'll create a PR to suppress this.

[0xdaryl]

eclipse/omr#6557 was created to suppress the note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1 warnings. This is an informational notification that there was a bug in earlier versions of gcc on AArch64 passing bitfields as parameters that a developer needs to be aware of if linking objects produced with different compiler levels. That isn't the case with OpenJ9, hence the warning will be suppressed.

[keithc-ca]

We will need to ensure that everything is compiled with a compatible compiler. Mixing GCC 8 (or any compiler that used the old ABI) with GCC 9 (or any compiler using the new ABI) will be problematic: that includes the JDK and even customer JNI code.

[pshipton]

Is JNI really a problem? JNI uses it's own ABI.

[keithc-ca]

JNI is an API, not an ABI. An ABI dictates things like the calling convention (including the rules for how registers are assigned for parameters and return values).

Perhaps I should have referred to JVMTI or just native code in general. As an example, a mismatch in ABI rules may break the linkage between callers and the implementation of JVMTI functions trafficking in jvmtiCapabilities.

[pshipton]

I thought JNI has an ABI of sorts, the calls are made via libffi, and afaik doesn't use bitfields.
jvmtiCapabilities uses a pointer to a structure containing bitfields.
Unless there is API using a bitfield parameter, I'm not sure there is a problem.
If there was a problem, everyone would need to recompile native code when moving from jdk11 to jdk17+.

[keithc-ca]

I think you're right in that there are no functions in the JNI or JVMTI APIs that involve a structure type with bit fields.

Reading https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88469 again, I see discussion about improperly computing the alignment of structures with bit-fields. The compiler might emit code that doesn't align an instance of jvmtiCapabilities sufficiently; passing the address of such a variable might trigger a misaligned access exception in the VM. Depending on the system configuration, the trap handler may compensate. If the system is configured not to compensate, the problematic (user) code would have to be recompiled.

[pshipton]

FWIW, I don't see any mention of recompiling native code in https://docs.oracle.com/en/java/javase/17/migrate/significant-changes-jdk-release.html

[vij-singh]

What would be the next step for this one, given it is in the 0.35 milestone?

[pshipton]

I still see the following:

https://openj9-jenkins.osuosl.org/job/Build_JDK17_aarch64_linux_Nightly/289
This is just one example, there are more in the build.

19:21:33  /home/jenkins/workspace/Build_JDK17_aarch64_linux_Nightly/omr/compiler/compile/OSRData.cpp: In member function 'void TR_OSRCompilationData::addInstruction(int32_t, TR_ByteCodeInfo)':
19:21:33  /home/jenkins/workspace/Build_JDK17_aarch64_linux_Nightly/omr/compiler/compile/OSRData.cpp:117:1: note: parameter passing for argument of type 'TR_ByteCodeInfo' changed in GCC 9.1
19:21:33    117 | TR_OSRCompilationData::addInstruction(int32_t instructionPC, TR_ByteCodeInfo bcInfo)
19:21:33        | ^~~~~~~~~~~~~~~~~~~~~

[pshipton]

eclipse/omr#6557 was created, but was never merged.

[pshipton]

There are still some cast warnings, one example below.

https://openj9-jenkins.osuosl.org/job/Build_JDK17_ppc64le_linux_Nightly/313

19:42:37  [ 92%] Building CXX object runtime/compiler/CMakeFiles/j9jit.dir/p/codegen/J9AheadOfTimeCompile.cpp.o
19:42:38  /home/jenkins/workspace/Build_JDK17_ppc64le_linux_Nightly/openj9/runtime/compiler/p/codegen/CallSnippet.cpp: In member function 'virtual uint8_t* TR::PPCUnresolvedCallSnippet::emitSnippetBody()':
19:42:38  /home/jenkins/workspace/Build_JDK17_ppc64le_linux_Nightly/openj9/runtime/compiler/p/codegen/CallSnippet.cpp:498:22: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
19:42:38    498 |          getNode() ? (uint8_t *)getNode()->getInlinedSiteIndex() : (uint8_t *)-1,
19:42:38        |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[0xdaryl]

We'll resolve these warnings for 0.36.

[hzongaro]

@dylanjtuttle, I know it's been some time, but do you know whether there were still any warnings being reported that would need to be handled before we could enable "warnings-as-errors" for OpenJ9 compiler builds? I see pull request #18313 - was that the only one remaining?

[hzongaro]

And I also see draft pull request #18614 for enabling it on Power with some compilers.

[dylanjtuttle]

As far as I remember, #18313 was the last pull request remaining before warnings as errors could be enabled on power for clang and gcc builds. Of course, it's possible that other warnings have been introduced since I was gone, but as long as there aren't too many I think it would be worth my time to clean them up so we can finally get warnings as errors turned on.

[vij-singh]

@pshipton Can we move this over to Milestone 0.49?
@dylanjtuttle and @luke-li-2003 are working on cleaning up the remaining warnings, but this will occur in the next round