RFR (L): 7088419 : Use x86 Hardware CRC32 Instruction with java.util.zip.CRC32 and java.util.zip.Adler32

David Chase david.r.chase at oracle.com
Wed Jun 26 12:31:00 PDT 2013


On 2013-06-26, at 2:17 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:

> For Doxygen parameters you should use @param:
> 
> http://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdparam
> 
> If we have a Doxygen method comment we don't need comments in header files:

I will look into Doxygen, but do we really "not need" those comments?
Some of us don't use Doxygen.

> src/share/vm/prims/jvm.cpp:
> 
> +   PUTPROP(props, "sun.zip.clmulSupported", X86_ONLY(!UseCRC32Intrinsics && UseAVX && UseCLMUL ? "true" :) "false");
> 
> When you say "No changes to JDK: this is all in the compiler." does it mean this review has no JDK changes or are there no JDK changes at all?

There are no JDK changes at all.
Probably the parameter-putting should go, since otherwise that means a new unwanted property.
And even if it doesn't, it should be called sun.zip.intrinsicCRC32 or some such.
And there's a bug in the parameter setting anyhow that I thought I had fixed -- should not negate UseCRC32Intrinsics.
(Somehow, I captured an old version of this file in my webrev; it's not relevant to running tests.)

> src/cpu/x86/vm/templateInterpreter_x86_32.cpp:
> 
> I thought I've changed this already but apparently I didn't:
> 
>      case Interpreter::java_lang_math_exp     : entry_point = ((InterpreterGenerator*)this)->generate_math_entry(kind);     break;
> 
> Can we please add a local variable for (InterpreterGenerator*)this (on all architectures)?

Would "ig_this" be happy-making?

David



More information about the hotspot-compiler-dev mailing list