RFR: 8308340: C2: Idealize Fma nodes [v3]
Fei Yang
fyang at openjdk.org
Thu Jul 20 09:46:46 UTC 2023
On Tue, 27 Jun 2023 16:05:54 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:
>> Fei Gao has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>>
>> - Merge branch 'master' into fg8308340
>> - Move check for UseFMA from c2compiler.cpp to Matcher::match_rule_supported in .ad files
>> - Merge branch 'master' into fg8308340
>> - 8308340: C2: Idealize Fma nodes
>>
>> Some platforms, like aarch64, ppc, and riscv, support fusing
>> `Math.fma(-a, b, c)` or `Math.fma(a, -b, c)` by generating
>> partially symmetric match rules like:
>>
>> ```
>> match(Set dst (FmaF src3 (Binary (NegF src1) src2)));
>> match(Set dst (FmaF src3 (Binary src1 (NegF src2))));
>> ```
>>
>> Since `Fma` is partially communitive, the patch is to convert
>> `Math.fma(-a, b, c)` to `Math.fma(b, -a, c)` in gvn phase,
>> making node patterns canonical. Then we can remove redundant
>> rules.
>>
>> Also, we should guarantee that C2 generates `Fma` nodes only on
>> platforms supporting `Fma` instructions before matcher, so we
>> can remove all `predicate(UseFMA)` for all `Fma` rules.
>>
>> After the patch, the code size of libjvm.so on aarch64 platform
>> decreased by 63.4k.
>>
>> The patch passed all tier 1 - 3 on aarch64 and x86 platforms.
>
> Looks good to me.
> You need second review.
> Thanks for your review @vnkozlov .
>
> I would appreciate it very much if some expert on ppc or riscv could help review it! Perhaps @RealFYang @reinrich
Hello, the RISC-V part looks fine from what this PR is supposed to do. And this has passed tier1-3 tests on linux-riscv64 platform. Note that I didn't check the shared code changes.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14576#issuecomment-1643608041
More information about the hotspot-compiler-dev
mailing list