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

[Aleksey Shipilev]

On Tue, 23 Jan 2024 11:18:43 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> This patch introduces `JitCompiler::isConstantExpression` which can be used to statically determine whether an expression has been constant-folded by the Jit compiler, leading to more constant-folding opportunities. For example, it can be used in `MemorySessionImpl::checkValidStateRaw` to eliminate the lifetime check on global sessions without imposing additional branches on other non-global sessions. This is inspired by `std::is_constant_evaluated` in C++.
>> 
>> Please kindly give your opinion as well as your reviews, thanks very much.
>
> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add more overloads

All right, this is very close :) I now have stylistic comments:

src/hotspot/share/classfile/vmIntrinsics.hpp line 912:

> 910:   do_intrinsic(_getAndSetInt,             jdk_internal_misc_Unsafe,     getAndSetInt_name, getAndSetInt_signature, F_R)       \
> 911:    do_name(     getAndSetInt_name,                                      "getAndSetInt")                                       \
> 912:    do_alias(    getAndSetInt_signature,                                 /*"(Ljava/lang/Object;JI)I"*/ getAndAddInt_signature) \

I don't think we need to do these formatting changes in this PR.

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.

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?

src/hotspot/share/opto/library_call.hpp line 2:

> 1: /*
> 2:  * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.

Unnecessary update?

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

PR Review: https://git.openjdk.org/jdk/pull/17527#pullrequestreview-1839148507
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463490470
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463493124
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463497227
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463497518