RFR: 8051725: Questionable if-conversion involving SETNE

Jasmine Karthikeyan duke at openjdk.org
Wed Apr 5 05:08:10 UTC 2023


Hi, I've created optimizations for the x86 lowering of `Conv2B` nodes, when followed immediately by an xor of 1. This pattern is fairly common, and can arise from both [cmov idealization](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/movenode.cpp#L241) and [diamond-phi optimization](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L1571). The optimization here is using the `sete` instruction instead of always using `setne` and flipping the bit with xor afterwards. According to the Intel optimization guide (pages 3-26 and 3-27), this sequence is preferred over `cmp $0, %src` as it prevents the need to encode the constant in the assembly sequence. A similar rule exists in the PPC backend, here: https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/ppc/ppc.ad#L10462. I've attached some performance testing but I think the real world improvements will be less significant- the motivation is primarily to decrease the amount of instructio
 ns that are generated, as that can help in cases where applications are I-Cache bound.


                                    Baseline                           Patch              Improvement
Benchmark                     Mode  Cnt  Score    Error Units     Score    Error  Units
Conv2BRules.testEquals0        avgt   10  47.566 ± 0.346 ns/op  /  37.904 ± 1.856  ns/op  + 22.6%
Conv2BRules.testNotEquals0     avgt   10  37.167 ± 0.211 ns/op  /  37.352 ± 1.529  ns/op  (unchanged)
Covn2BRules.testEquals1        avgt   10  35.059 ± 0.280 ns/op  /  34.847 ± 0.160  ns/op  (unchanged)
Covn2BRules.testEqualsNull     avgt   10  56.768 ± 2.600 ns/op  /  46.916 ± 0.308  ns/op  + 19.0%
Conv2BRules.testNotEqualsNull  avgt   10  47.447 ± 1.193 ns/op  /  46.974 ± 0.218  ns/op  (unchanged)


This change also cleans up some code relating to `Assembler::set_byte_if_not_zero`, as that function duplicates behavior with `Assembler::setne`. The 32-bit only version of that method is never called as the only other usage is in the C1 LIR assembler, which is also guarded behind an 64-bit check so I opted to remove it entirely and replace usages with `Assembler::setne`. Reviews would be greatly appreciated!

Testing: tier1-2 on linux x64, GHA

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

Commit messages:
 - Fix whitespace and add bug tag to IR test
 - Merge branch 'master' into conv2b-x86-lowering
 - Merge branch 'master' into conv2b-x86-lowering
 - Merge branch 'master' into conv2b-x86-lowering
 - Merge branch 'master' into conv2b-x86-lowering
 - Add flipped versions of Conv2B rules to decrease generated code complexity

Changes: https://git.openjdk.org/jdk/pull/13345/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=13345&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8051725
  Stats: 265 lines in 7 files changed: 251 ins; 11 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/13345.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13345/head:pull/13345

PR: https://git.openjdk.org/jdk/pull/13345


More information about the hotspot-compiler-dev mailing list