RFR: 8309978: [x64] Fix useless padding

Christian Hagedorn chagedorn at openjdk.org
Wed Jun 14 07:07:56 UTC 2023


On Wed, 14 Jun 2023 04:42:22 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> Fixed typo in `IntelJccErratum::compute_padding()`.
> 
> Due to the typo (`mach` instead of `next`) useless padding could be generated because size of `mach` instruction (which is `cmp` in this case and big)  counted twice. As result combined size most likely cross 32-bytes cache line boundary and padding is generated to avoid that.
> 
> 
> 030    B2: # out( B4 B3 ) <- in( B1 ) Freq: 0.999999 
>        nop # 16 bytes pad for loops and calls 
> 040    cmpb [R12 + R10 << 3 + #144] (compressed oop addressing), #42 
> 049    jle,s B4 P=0.667944 C=6785.000000
> 
> Note: only some x86 CPUs are [affected](https://github.com/openjdk/jdk/blob/ba837b4bfa2dea85653d8a8fccd0817a569b4378/src/hotspot/cpu/x86/vm_version_x86.cpp#L1957).
> 
> For new IR test to work I moved `PHASE_FINAL_CODE` IR print inside `PhaseOutput` scope because padding nodes (`NOP` mach nodes) are present only in this phase IR.
> 
> Added new IR test. Tested tier1-3, xcomp, stress.

Otherwise, the fix looks good. Thanks for adding an IR test!

test/hotspot/jtreg/compiler/c2/irTests/TestPadding.java line 50:

> 48:             test(i);
> 49:             tpf.b1++; // to take both branches in test()
> 50:         }

`test_runner()` will be invoked 2000 times (default warm-up) before the explicit compilation request of `test()` by the IR framework. So, this loop will run 2000 * 11000 times. Do you need that many iterations or can the loop be removed such that we only have 2000 warm-up iterations? I.e. something like:

    @Run(test = "test")
    public static void test_runner() {
        tpf = new TestPadding();
        test(42);
        tpf.b1++; // to take both branches in test()

    }

If you need more iterations, you could still specify `@Warmup(12345)` at `test_runner()` to get more profiling in before compilation of `test()`.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14461#pullrequestreview-1478618687
PR Review Comment: https://git.openjdk.org/jdk/pull/14461#discussion_r1229110689


More information about the hotspot-compiler-dev mailing list