On Tue, 23 Jan 2024 15:44:40 GMT, Aleksey Shipilev <sh...@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
