RFR (S) 8131682: C1 should use multibyte nops everywhere

Dean Long dean.long at oracle.com
Sat Jul 18 07:51:03 UTC 2015


I think we should distinguish the different uses and treat them accordingly:

1) padding nops for patching, executed

We need to be careful about inserting a fat nop here, if later patching 
overwrites only part of the fat nop, resulting in an illegal intruction.

2) padding nops for patching, never executed

It should be safe insert a fat nop here, but there's no point if the nops are not reachable and never executed.


3) alignment nops, never patched, executed

Fat nops are fine, but on some CPUs branching may be even better, so I 
suggest using align() for this, and letting align() decide what to 
generate.  The change in check_icache() could use a version of align 
that takes the target  offset as an argument:

348 align(CodeEntryAlignment,__ offset() + ic_cmp_size);

4) alignment nops, never patched, never executed

Doesn't matter what we emit here, but we might as well make it 
understandable by humans using a debugger.


I believe the patching nops in c1_CodeStubs_x86.cpp and 
c1_LIRAssembler.cpp are patched concurrently while the code is running, 
not at a safepoint, so it's not clear to me if it's safe to use fat nops 
on x86.  I would consider those changes unsafe on x86 without further 
analysis of what happens during patching.

dl

On 7/17/2015 6:29 AM, Aleksey Shipilev wrote:
> Hi there,
>
> C1 is not very good at inlining and intrisifying methods, and hence the
> call performance is important there. One nit that we can see in the
> generated code on x86 is that C1 uses the single-byte nops, even for
> long nop strides.
>
> This improvement fixes that:
>    https://bugs.openjdk.java.net/browse/JDK-8131682
>    http://cr.openjdk.java.net/~shade/8131682/webrev.00/
>
> Testing:
>    - JPRT -testset hotspot on open platforms
>    - eyeballing the generated assembly with -XX:TieredStopAtLevel=1
>
> (I understand the symmetric change is going to be needed in closed
> parts, but let's polish the open part first).
>
> Thanks,
> -Aleksey
>



More information about the hotspot-compiler-dev mailing list