RFR: 8303278: Imprecise bottom type of ExtractB/UB

[Jatin Bhateja]

On Tue, 21 Mar 2023 02:35:25 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Yes x86 does not handle signed extension correctly. `pextrb` and `pextrw` zeroes the upper bits instead of signed extending them. A simple fix is to add `movsx` after those.
> 
> >https://github.com/openjdk/jdk/blob/bbca7c3ede338a04d140abfe3e19cb27c628a0f5/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L2247

![image](https://user-images.githubusercontent.com/59989778/226945245-85115014-9b1b-4cf0-b8e7-53b34d0e4f57.png)

- As can be seen above C2 creates ExtractS node with TypeInt::SHORT which is then succeeded by  ConvI2L  node since expander returns a long value.
- Return value is again down casted back short value though an explicit cast operation in java code.

![image](https://user-images.githubusercontent.com/59989778/226948646-7b123472-23c3-4b2d-9f53-a8a6f9534802.png)
  

Currently C2 folds following IR sequence 


ExtractS  -> ConvI2L -> ConvL2I ->  LShift (16) -> RShift(16)                   ==>  ExtractS -> return_value
    [Inline expander]    [ Java side down cast long to short -> l2i + i2s]  


Because ideal type of ExtractS is TypeInt::SHORT hence entire chain of conversion operations are folded by compiler during idealizations.

There are two ways to address this issue:-
1)  We can set the ideal type of ExtractS node to TypeInt::INT as is done for ExtractB/UB since subsequent java code will handle down casting to generate correct result. This will also fix [JDK-8303508](https://bugs.openjdk.org/browse/JDK-8303508).
2)  Enforce strict semantics of ExtractS/B IR node and thus backends should emit an explicit sign extension instruction movsx for sub-word types.

I agree with @merykitty that 2) solution looks more robust since it will enforce accurate semantics of ExtractS IR.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/13070#issuecomment-1479779537