Skip to content

Conversation

ehsankianifar
Copy link
Contributor

Jit test harness initialize a OMRPortLibrary but never add it to the compiler environment. Some platforms like z rely on port library for some operations like getting the cpu features. As a result, z always defaults on the minimum supported version to get cpu features in evaluators while running compiler tests which does not match the actual cpu and cause false failures.
This change add the OMRPortLibrary to the test compiler environment.

@ehsankianifar
Copy link
Contributor Author

While I was working on some vector evaluators, I noticed we have some jit test to cover those but we never run it on z platform.
The reason is we use OMRCPU methods to check if features supported or not and in those methods we pass the OMRPortLibrary to get the cpu information. If no OMRPortLibrary is provided, it uses the minimum supported CPU which does not support many features.
First I created an issue [1] to bump up the minimum cpu but it dest not seem a correct approach since it might be different from the actual cpu.
Looking closer in jit test harness implementation, I noticed that we initialize an OMRPortLibrary but never add it to test compiler environment.
I opened this PR to add required procedure to add the OMRPortLibrary to the compiler environment.
This change will also close an existing issue [2]
[1]: #7838
[2]: #1843

@ehsankianifar
Copy link
Contributor Author

I enabled a few of tests that were disabled on z because of this issue and find a few tests are failing with this change. I disabled those tests and created issues to fix the failures:
1: VectorTest.VInt8BitSelect #7910
2: CompareTest/UInt32Compare #7911

@ehsankianifar
Copy link
Contributor Author

@r30shah I want to know your expert opinion on this to see if there are any unwanted consequences or not. I also want to make sure other platforms are okay with this change since the scope of this is cross-platform!

@ehsankianifar
Copy link
Contributor Author

What test pipeline I can use to test this on all platforms?

@r30shah
Copy link
Contributor

r30shah commented Aug 28, 2025

jenkins build all

@r30shah
Copy link
Contributor

r30shah commented Aug 29, 2025

@ehsankianifar There are series of failure with the proposed change. Though I do agree having creating and using port lib for test to enable the testing of newer features through OMR tests, I think this kind of work requires an issue documenting what problem we are facing with not using port lib and how are you planning to address it - Not to mention we should not have failures so let's start with the issue so we can discuss on possible solution.

@ehsankianifar
Copy link
Contributor Author

jenkins build all

1 similar comment
@r30shah
Copy link
Contributor

r30shah commented Sep 15, 2025

jenkins build all

@ehsankianifar
Copy link
Contributor Author

Instead of changing the JitTest I decided to add a JitWithPortTest class. The JitTest class has not changed therefore the tests driven from it should not be effected by the changes in this PR. We can gradually switch tests to drive from JitWithPortTest and fix the tests if there is any failure.
I enabled some tests That where disabled on IBM Z platform and all of them pass on my local environment.

@ehsankianifar
Copy link
Contributor Author

I observed some failures on jenkins builds on IBM Z platforms and RISC_V64.
riscv64:

09:37:01  28: �[0;32m[----------] �[m4800 tests from ArithmeticTest/DoubleArithmetic
09:37:21  28/29 Test #28: comptest ..........................***Exception: Illegal118.55 sec

zlinux:

09:06:29  29: �[0;31m[  FAILED  ] �[mTarnaryFloatNaNInfTest/TernaryDataDrivenFloatTest.TernaryVector128FloatTest/2, where GetParam() = (34, 512-byte object <7F-F0 00-00 00-00 00-00 7F-F8 00-00 00-00 00-00 7F-F8 00-00 00-00 00-00 7F-F8 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00>)
09:06:29  29: �[0;31m[  FAILED  ] �[mTarnaryFloatNaNInfTest/TernaryDataDrivenFloatTest.TernaryVector128FloatTest/3, where GetParam() = (34, 512-byte object <7F-F0 00-00 00-00 00-00 7F-F8 00-00 00-00 00-00 FF-F0 00-00 00-00 00-00 7F-F8 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00>)
09:06:29  29: �[0;31m[  FAILED  ] �[mTarnaryFloatTest/TernaryDataDrivenFloatTest.TernaryVector128FloatTest/0, where GetParam() = (34, 512-byte object <40-5B 80-00 00-00 00-00 40-14 00-00 00-00 00-00 40-50 40-00 00-00 00-00 40-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00>)

The same tests that fail on zLinux pass on my local setup. @r30shah is there a way to book these jenkin machines to investigate?

@ehsankianifar
Copy link
Contributor Author

ehsankianifar commented Sep 15, 2025

I think the test setup is not providing the correct arguments to the tree! We expect 32bit 'float' type but the test provides 64bit double type instead! Might be a parallelism issue and float test data gets mixed with double!

@ehsankianifar
Copy link
Contributor Author

The jenkins environment is using GCC 4.8

08:52:12  -- The CXX compiler identification is GNU 4.8.5
08:52:13  -- The C compiler identification is GNU 4.8.5

@r30shah
Copy link
Contributor

r30shah commented Sep 15, 2025

The same tests that fail on zLinux pass on my local setup. @r30shah is there a way to book these jenkin machines to investigate?

You can open up issue internally to runtimes/infrastructure to get access to the machine (Tag @AdamBrousseau )

@ehsankianifar
Copy link
Contributor Author

I think the problem is that float_t was used as the data type in this test instead of float. Probably that is why I get single precision on my local environment but we have double precision on jenkins!

@ehsankianifar ehsankianifar force-pushed the AddOmrPortLibToJitTestEnvironment branch from 6e4cb0f to fb20afe Compare September 16, 2025 14:50
Jit test harness initialize a OMRPortLibrary but never add it to the
compiler environment. Some platforms like z rely on port library for
some operations like getting the cpu features. As a result, z always
defaults on the minimum supported version to get cpu features in
evaluators while running compiler tests which does not match the actual
cpu and cause false failures.
This change add the OMRPortLibrary to the test compiler environment.

signed-off-by: Ehsan Kiani Far <[email protected]>
@ehsankianifar ehsankianifar force-pushed the AddOmrPortLibToJitTestEnvironment branch from 2bd626d to 6591ad9 Compare September 16, 2025 17:23
@ehsankianifar ehsankianifar marked this pull request as ready for review September 16, 2025 17:23
@ehsankianifar
Copy link
Contributor Author

@r30shah can I ask you to trigger the tests again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants