UDivI/L (generated by the Integer.divideUnsigned() intrinisic) and UModI/L (generated by the Integer.remainderUnsigned() intrinsic) miss some optimizations done for (signed) DivI/L and ModI/L.
For example:
- x % 1 is optimized to 0 while Integer.remainderUnsigned(iFld, 1) is not.
- The control input is optimized away for DivI/L and ModI/L if the divisor cannot be zero while this is not done for UDivI/L and UModI/L. Note that the original report below wrongly assumed control *is* removed for the unsigned divisions and thus suffer from the same problems already fixed for the signed divisions with JDK-8336729 and related bugs. When this optimization is added for the unsigned divisions, we need to ensure that this won't happen (see related bugs and tests which can be extended to unsigned).
This is not a complete list of missing optimizations. The goal of this RFE is to compare all Ideal/Identity/Value methods of the signed DivI/L and ModI/L nodes with the unsigned ones and figure out which optimizations can be applied to the unsigned versions as well. Ideally, we extract the shared logic into a separate class. We might not want to add a common super class for DivI/L and for ModI/L or make one a sub class of the other as this introduces some coupling and might have have unforeseeable consequences in existing code (e.g. is_Div() now also accepts UDiv nodes etc.). It might be cleaner to just have a separate class doing the logic, as for example done with IntegerTypeMultiplication:
https://github.com/openjdk/jdk/blob/f0b130e54f33d3190640ce33c991e35f27e9f812/src/hotspot/share/opto/mulnode.cpp#L346
Additionally, we should introduce IR framework tests that ensure the correctness of the new and existing (if not already exist) optimizations.
======================
Original report:
C2: PhaseIterGVN::no_dependent_zero_check() should handle unsigned div/mod
Summary:
Came up in the review for:
https://github.com/openjdk/jdk/pull/18377
"Looking at PhaseIterGVN::no_dependent_zero_check, I noticed that UDiv[I/L]Node and UMod[I/L]Node are not handled but I think they should. I think this was missed when these nodes where added by JDK-8282221. One can probably extend @chhagedorn's test from JDK-8259227 to trigger the same issue."