Add missing conversion for load generated by UnsafeFastPath #3218
Conversation
|
@fjeremic @andrewcraik Can I have a review? |
| @@ -560,7 +560,7 @@ int32_t TR_UnsafeFastPath::perform() | |||
| else | |||
| { | |||
| //This is a load | |||
| if (isByIndex && node->getDataType() != type) | |||
| if (node->getDataType() != type) | |||
There was a problem hiding this comment.
There seems to be an assert below guarding for future modifications. Are you certain we won't hit this assert? Note it's not a FATAL assert so it won't fire by default (outside of debug builds).
There was a problem hiding this comment.
Not for Unsafe methods. Is there similar problem to JITHelpers methods like getByteFromArray? The method return byte but in trees we use icallx, so a conversion is needed on the load.
There was a problem hiding this comment.
Now that you mention, I'll update the assertion message to not limit to JITHelpers.
There was a problem hiding this comment.
@fjeremic Have my comments answered your questions? Any more concerns?
Convert the result of load if the result type doesn't match the Unsafe call's return type. The conversion is needed for accessing arrays whose element type is narrower than 32 bits. Signed-off-by: Liqun Liu <liqunl@ca.ibm.com>
|
Added change to also fix store. |
|
CI should be sufficient - merging |
Convert the result of load if the result type doesn't match the Unsafe
call's return type. The conversion is needed for accessing arrays whose
element type is narrower than 32 bits.
Signed-off-by: Liqun Liu liqunl@ca.ibm.com