RFR: 8336860: x86: Change integer src operand for CMoveL of 0 and 1 to long [v3]
Emanuel Peter
epeter at openjdk.org
Fri Aug 30 12:53:22 UTC 2024
On Mon, 5 Aug 2024 16:48:52 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:
>> Hi all,
>> This patch fixes `cmovL_imm_01*` instructions matching against an integer immediate 1 instead of a long immediate 1. I noticed while looking at the backend implementation of CMove that the rules specify `immI_1` instead of `immL1`, which means that the instructions can't be matched and instead falls through to the base case. I added a small benchmark and got these results:
>>
>> Baseline Patch Improvement
>> Benchmark Mode Cnt Score Error Units Score Error Units
>> BasicRules.cmovL_imm_01 avgt 15 259.073 ± 5.806 ns/op 231.108 ± 2.730 ns/op (+ 11.41%)
>>
>> Thoughts and reviews would be appreciated!
>
> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
>
> Add IR test for codegen
Otherwise, it looks good to me!
test/hotspot/jtreg/compiler/c2/irTests/CMoveLConstants.java line 33:
> 31: * bug 8336860
> 32: * @summary Verify codegen for CMoveL with constants 0 and 1
> 33: * @requires os.simpleArch == "x64"
Hmm. How about only requiring `x64` in the IR rules, but running the test on all platforms? That way we can at least check correct results.
-------------
Marked as reviewed by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20275#pullrequestreview-2272221017
PR Review Comment: https://git.openjdk.org/jdk/pull/20275#discussion_r1738593057
More information about the hotspot-compiler-dev
mailing list