On Tue, 23 Jan 2024 17:21:47 GMT, Quan Anh Mai <[email protected]> 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 similar to
>> `__builtin_constant_p` in GCC and clang.
>>
>> 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:
>
> ident
A few more stylistic comments :)
Still thinking the better home for these might be just
`jdk.internal.misc.VM`... But I would not insist, if others are happy.
src/java.base/share/classes/jdk/internal/misc/JitCompiler.java line 56:
> 54: */
> 55: @IntrinsicCandidate
> 56: public static boolean isCompileConstant(boolean expr) {
Here and in other places: probably not `expr`, but just `val` or something?
src/java.base/share/classes/jdk/internal/misc/JitCompiler.java line 119:
> 117: * @see #isCompileConstant(boolean)
> 118: */
> 119: @IntrinsicCandidate
Note how the Java entry for MH intrinsic we have replaced had `@Hidden`. These
methods should have `@Hidden` too then? Probably applies to other entries too.
-------------
PR Review: https://git.openjdk.org/jdk/pull/17527#pullrequestreview-1839475907
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463705907
PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1463703771
