RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v3]

Quan Anh Mai qamai at openjdk.org
Tue Jan 23 16:56:29 UTC 2024


On Tue, 23 Jan 2024 15:44:40 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add more overloads
>
> src/hotspot/share/classfile/vmIntrinsics.hpp line 927:
> 
>> 925:                                                                                                                               \
>> 926:   do_class(jdk_internal_misc_JitCompiler, "jdk/internal/misc/JitCompiler")                                                    \
>> 927:   do_intrinsic(_isConstantExpressionZ,    jdk_internal_misc_JitCompiler,isConstantExpression_name, bool_bool_signature, F_S)  \
> 
> It would be cleaner to follow the current naming for existing intrinsic:
> 
> 
>   do_intrinsic(_isCompileConstant, java_lang_invoke_MethodHandleImpl, isCompileConstant_name, isCompileConstant_signature, F_S) \
>    do_name(     isCompileConstant_name,                          "isCompileConstant")                                   \
>    do_alias(    isCompileConstant_signature,                      object_boolean_signature)                             \
> 
> 
> I.e. rename `isConstantExpression` -> `isCompileConstant`. It clearly communicates that we are not dealing with expressions as arguments, and that we underline this is the (JIT) _compile_ constant, not just a constant expression from JLS 15.28 "Constant Expressions".
> 
> Maybe even replace that `MHImpl` method with the new intrinsic.

Yes you are right, I have renamed it to `isCompileConstant`.

> src/hotspot/share/opto/c2compiler.cpp line 727:
> 
>> 725:   case vmIntrinsics::_storeStoreFence:
>> 726:   case vmIntrinsics::_fullFence:
>> 727:   case vmIntrinsics::_isConstantExpressionZ:
> 
> Move this closer to `vmIntrinsics::_isCompileConstant:`, if not outright replace it?

I have replaced `MHImpl::isCompileConstant` with the new one.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463617016
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463616039


More information about the core-libs-dev mailing list