Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Z: Don't pass sreg to ETND #19495

Merged
merged 1 commit into from
May 14, 2024
Merged

Conversation

Spencer-Comin
Copy link
Contributor

@Spencer-Comin Spencer-Comin commented May 14, 2024

ETND does not take a source register.

While investigating #14054 I ran into a SIGILL failure. The illegal instruction was encoded as B2EC0011. From the jitdump, this corresponds to ETND GPR1,GPR1. ETND only operates on one register encoded in bits 24-27 of the instruction, and bits 28-31 should be left 0 [1]. ETND is encoded by TR::S390RRInstruction::generateBinaryEncoding(), where the second register operand, when it exists, is encoded into bits 28-31 [2], leading to an incorrectly encoded ETND. By not unnecessarily passing codeReg as a source register when the ETND is generated, we avoid the incorrect encoding.

[1] https://hurgsa.ibm.com:7191/projects/h/hlasmhursley/public/SA22-7832-13/0705103.htm
[2] https://github.com/eclipse/omr/blob/b9a6cccd97d6da637eab6f360be8dc4f9bd194ad/compiler/z/codegen/S390Instruction.cpp#L1348-L1352

ETND does not take a source register.

Signed-off-by: Spencer Comin <[email protected]>
@Spencer-Comin
Copy link
Contributor Author

@r30shah FYI

@Spencer-Comin
Copy link
Contributor Author

@r30shah
Copy link
Contributor

r30shah commented May 14, 2024

Thanks @Spencer-Comin .
I wonder issue seen in #9876 is same ?
Please fix the line endings and I will launch the builds

@AdamBrousseau
Copy link
Contributor

Jenkins line endings check

@r30shah
Copy link
Contributor

r30shah commented May 14, 2024

Jenkins test sanity zlinux jdk21

Copy link
Contributor

@r30shah r30shah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@r30shah
Copy link
Contributor

r30shah commented May 14, 2024

@VermaSh Do you have log file / core dump archived for any of the failure for #9876 which can confirm if it is same issue or not ?

@r30shah r30shah merged commit f4d4782 into eclipse-openj9:master May 14, 2024
5 checks passed
@VermaSh
Copy link
Contributor

VermaSh commented May 16, 2024

@r30shah It does seem to be same issue, b2ec0066 should have been b2ec0060 [1-2]. Nice catch! @Spencer-Comin

[1] Register context
(kca) whatis reg
$r0   = 0x0000000000000016 (22)
$r1   = 0x000003ff9387d238 Ptr J9JavaVM
$r2   = 0x000000000000000e (14)
$r3   = 0x000003ff92f910a0 Ptr Unknown
$r4   = 0x000003ff75a80284 {com/ibm/jvmti/tests/decompileAtMethodResolve/ResolveTest1Super.test} +394
$r5   = 0x000003ff8c339600 MemBlk - decomp.cpp:1167 +72
$r6   = 0x0000000000000000 Null
$r7   = 0x000003ff8c095790 MemBlk - trcmain.c:997 +8
$r8   = 0x000003ff9387e580 Ptr Unknown
$r9   = 0x000003ff9242aa84 Ptr Unknown
$r10  = 0x000003ff92893f94 Ptr Unknown
$r11  = 0x000003ff9387d468 Ptr Unknown
$r12  = 0x000003ff9387d470 Ptr Unknown
$r13  = 0x0000000000065a00 J9VMThread
$r14  = 0x000003ff9387d638 Ptr Unknown
$r15  = 0x000003ff9387d2a8 Ptr Unknown
$addr = 0x000003ff75a8028c {com/ibm/jvmti/tests/decompileAtMethodResolve/ResolveTest1Super.test} +402
$bea  = 0x000003ff92f02ef8 Ptr Unknown
[2] Failing instruction
(kca) x/5i 0x000003ff75a8028c-4 
0x3ff75a80288 {com/.../ResolveTest1Super.test} +398         -1:0         b2ec invalid instruction // aload0
0x3ff75a8028a {com/.../ResolveTest1Super.test} +400                      0066 invalid instruction
0x3ff75a8028c {com/.../ResolveTest1Super.test} +402                      a76e0000     chi       %r6, 0
0x3ff75a80290 {com/.../ResolveTest1Super.test} +406                 *    a7840006     je        0x3ff75a8029c C>> +418
0x3ff75a80294 {com/.../ResolveTest1Super.test} +410                 |    a7690100     lghi      %r6, 0x100

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.

5 participants