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.

Sign up for GitHub

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

Jump to bottom

Fix immediate value field for addis and lis instruction on Power #9612

Closed
IBMJimmyk opened this issue May 19, 2020 · 1 comment · Fixed by #9613
Closed

Fix immediate value field for addis and lis instruction on Power #9612

IBMJimmyk opened this issue May 19, 2020 · 1 comment · Fixed by #9613
Labels
arch:power comp:jit

Comments

@IBMJimmyk
Copy link
Contributor

addis and lis accept a signed 16 bit value as an immediate value. They are passed into generateTrg1Src1ImmInstruction or generateTrg1ImmInstruction as signed 32 bit values. In some edge cases, the sign extension is not handled properly. In most cases this is due to the value being incremented after sign extension. In other cases it was due to an unnecessary mask clearing the upper 16 bits.

One case in genInstanceOfOrCheckCastSuperClassTest was generating the wrong instruction. addis should be used instead of lis to reconstruct the 32 bit immediate value to be added.
@IBMJimmyk IBMJimmyk mentioned this issue May 19, 2020
Merged
@IBMJimmyk
Copy link
Contributor Author

One of the edge cases is trying to add 0x7FFFFFFF to something in a register (for example in r3). Both are 32bit values.

An example of code that might be generated is:

addis r3, r3, 0x8000
addi r3, r3, 0xFFFF

Note how the top half 0x7FFF is changed to 0x8000 to compensate for how the bottom half 0xFFFF get sign extended when it get added in.

This causes a problem because 0x00007FFF is stored as a 32 bit values before being passed into generateTrg1Src1ImmInstruction. When it get incremented to 0x00008000 it is no longer correct. The sign needs to be fixed so it is 0xFFFF8000 before being passed into generateTrg1Src1ImmInstruction.

IBMJimmyk added a commit to IBMJimmyk/openj9 that referenced this issue Jun 11, 2020
@IBMJimmyk
Fixed imm value for addis and lis
992dfa1 
addis and lis accept a signed 16 bit value as an immediate. They are passed
into generateTrg1Src1ImmInstruction as signed 32 bit values. In some edge
cases, the sign extension was not handled properly. In most cases this was
due to the value being incremented after sign extension. In other cases it
was due to an unnecessary mask clearing the upper 16 bits.

One case in genInstanceOfOrCheckCastSuperClassTest was handled incorrectly.
addis should be used instead of lis to reconstruct the 32 bit immediate value
to be added.

For cases where the TOC offset is unusually large, I put in an assert instead
of generating instructions. The reasoning was TOC offsets of this size should
never occur so those code paths would never be tested. Also TOC offsets of
that size are likely an indication of a problem elsewhere.

Closes: eclipse-openj9#9612
Signed-off-by: jimmyk <jimmyk@ca.ibm.com>
IBMJimmyk added a commit to IBMJimmyk/openj9 that referenced this issue Jun 19, 2020
@IBMJimmyk
Fixed imm value for addis and lis
147c1ab 
addis and lis accept a signed 16 bit value as an immediate. They are passed
into generateTrg1Src1ImmInstruction as signed 32 bit values. In some edge
cases, the sign extension was not handled properly. In most cases this was
due to the value being incremented after sign extension. In other cases it
was due to an unnecessary mask clearing the upper 16 bits.

One case in genInstanceOfOrCheckCastSuperClassTest was handled incorrectly.
addis should be used instead of lis to reconstruct the 32 bit immediate value
to be added.

For cases where the TOC offset is unusually large, I put in an assert instead
of generating instructions. The reasoning was TOC offsets of this size should
never occur so those code paths would never be tested. Also TOC offsets of
that size are likely an indication of a problem elsewhere.

Closes: eclipse-openj9#9612
Signed-off-by: jimmyk <jimmyk@ca.ibm.com>
gita-omr pushed a commit to gita-omr/openj9 that referenced this issue Jun 19, 2020
@IBMJimmyk @gita-omr
Fixed imm value for addis and lis
3e13da4 
addis and lis accept a signed 16 bit value as an immediate. They are passed
into generateTrg1Src1ImmInstruction as signed 32 bit values. In some edge
cases, the sign extension was not handled properly. In most cases this was
due to the value being incremented after sign extension. In other cases it
was due to an unnecessary mask clearing the upper 16 bits.

One case in genInstanceOfOrCheckCastSuperClassTest was handled incorrectly.
addis should be used instead of lis to reconstruct the 32 bit immediate value
to be added.

For cases where the TOC offset is unusually large, I put in an assert instead
of generating instructions. The reasoning was TOC offsets of this size should
never occur so those code paths would never be tested. Also TOC offsets of
that size are likely an indication of a problem elsewhere.

Closes: eclipse-openj9#9612
Signed-off-by: jimmyk <jimmyk@ca.ibm.com>
@gita-omr gita-omr mentioned this issue Jun 19, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arch:power comp:jit
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed imm value for addis and lis IBMJimmyk/openj9
2 participants
@gacholio @IBMJimmyk