RFR: 8341781: Improve Min/Max node identities

[Jasmine Karthikeyan]

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

>> test/hotspot/jtreg/compiler/c2/irTests/TestMinMaxIdentities.java line 116:
>> 
>>> 114: 
>>> 115:     @Test
>>> 116:     @IR(applyIfPlatform = { "riscv64", "false" }, phase = { CompilePhase.BEFORE_MACRO_EXPANSION }, counts = { IRNode.MIN_L, "1" })
>> 
>> Can you add a comment here why we cannot apply the rules for riscv?
>
> 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?

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

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