RFR: 8299546: C2: MulLNode::mul_ring() wrongly returns bottom type due to casting errors with large numbers
Quan Anh Mai
qamai at openjdk.org
Tue Jan 10 14:49:54 UTC 2023
On Tue, 10 Jan 2023 14:16:52 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
>> I would suggest we do a 128-bit multiplication or a 64-bit high multiplication instead. This will help constant folding of `MulHiNode`s and also `MulLNode` even if the multiplication overflows. Also, a high multiplication is likely to be cheaper than 4 division.
>>
>> Thanks.
>
>> I would suggest we do a 128-bit multiplication or a 64-bit high multiplication instead. This will help constant folding of `MulHiNode`s and also `MulLNode` even if the multiplication overflows. Also, a high multiplication is likely to be cheaper than 4 division.
>
> Thanks for your suggestion. I'm not sure if I understand this correctly, though. How would this help for `MulHiNode`? And if we have an overflow, I'm not sure how we can get a better type then bottom for the `Mul` node since we cannot have multiple ranges for a type. Could you explain your idea in more detail?
>
> But if we are just talking about the implementation, how could we do a 128-bit multiplication inside the VM? Using `__int128_t` (I haven't seen any usage of that though - I guess it is not supported)? However, I think we should not worry too much about the performance given that these are only 4 divisions and we are in the context of C2 compiling a method.
@chhagedorn I think we can do similar to what our core library is doing
https://github.com/openjdk/jdk/blob/8b0133f2760f67cd968528c041a600408cc26978/src/java.base/share/classes/java/lang/Math.java#L1399
This helps in the sense that we currently do not have a 64-bit high multiplication inside the VM, so having one makes adding a more effective `MulHiNode::Value` become trivial. Regarding `MulLNode::Value`, even if the multiplication overflows, I think we can still reason about the value of the product if all the high parts are the same. The same goes for `MulINode::Value` where we can cast them to `jlong` instead of `double`.
Thanks.
-------------
PR: https://git.openjdk.org/jdk/pull/11907
More information about the hotspot-compiler-dev
mailing list