RFR: 8051725: Questionable if-conversion involving SETNE [v3]

Jasmine Karthikeyan jkarthikeyan at openjdk.org
Wed Apr 19 04:30:39 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 instruct
 ions 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)
> Conv2BRules.testEquals1        avgt   10  35.059 ± 0.280 ns/op  /  34.847 ± 0.160  ns/op  (unchanged)
> Conv2BRules.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

Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:

  Remove Conv2B from backend as it's macro expanded now

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/13345/files
  - new: https://git.openjdk.org/jdk/pull/13345/files/ee468b9e..59a68a10

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=13345&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13345&range=01-02

  Stats: 648 lines in 11 files changed: 91 ins; 555 del; 2 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