Fixed imm value for addis and lis #9613
Conversation
|
@gita-omr You might be interested in this. |
|
@aviansie-ben fyi |
There was a problem hiding this comment.
After thinking on this a while longer, I realized that truncating the return value of HI_VALUE doesn't solve the problem unless the upper 32 bits of the result are ignored and can result in incorrect code being generated, as I explained in eclipse-omr/omr#5213 (comment). Another solution would be required in instances where the upper 32 bits are used.
|
I changed the code to specifically detect cases like Prior to any of my changes, it would have just triggered the assert. Without the assert, it would have generated incorrect code. |
|
I changed how large TOC offsets ( |
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>
|
I rebased on the latest openj9 and squashed my commit together. |
|
@gita-omr Do you have any other comments or requests for changes? |
|
Jenkins test sanity aix,plinux jdk8,jdk11 |
|
Jenkins test extended aix,plinux jdk8,jdk11 |
|
This change isn't actually merged for 0.21, but master instead, so I'm removing the milestone tag. |
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.
Closes: #9612
Signed-off-by: jimmyk jimmyk@ca.ibm.com