RFR: 8341781: Improve Min/Max node identities

[Christian Hagedorn]

On Fri, 11 Oct 2024 05:00:06 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> This is a good call, the IR rules don't apply to RISC-V because it doesn't have support for CMoves so the MinL/MaxL nodes aren't made at all. Since `Math.min/max(LL)` isn't intensified it first needs to be matched into CMove, then Min/Max, and then the identity needs to be called. Since #20098 implements the intrinsic we could remove the special casing after it's merged. I've added a comment to the source code as well.
>
> On closer inspection it seems that because of the CMove cost model, the outer min/max operation doesn't get turned into a CMove so the Long IR rules don't reliably get matched anywhere. It must have slipped through the cracks because of the way that my IR rules were structured, I only realized this after I added the compile phase to the other 2 rules. I think for this to work it would need the intrinsics from the other PR. Do you think we should continue with this PR with the Long cases disabled and enable it afterwards, or should we wait for #20098 to be merged?

Thanks for sharing more details. I think it's perfectly fine to still add them now but leave them disabled with a reference to JDK-8307513 since you already wrote them.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21439#discussion_r1796513108