Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Thu, 25 Jan 2024 14:48:16 GMT, Maurizio Cimadamore
wrote:
> I don't 100% buy the `MethodHandleImpl` analogy. In that case the check is
> not simply used to save a branch, but to spare spinning of a completely new
> lambda form.
Doing this to save a few intructions would not likely to worth the hassle
outside the _really performance critical paths_, but even then it might be
useful for hot JDK code. On larger examples, you can avoid memory accesses,
allocations, etc. by coding up the constant-foldable path that you know
compiler would not be able to extract when propagating constants through the
generic code. For example, giving quantitative substance to my previous example:
diff --git a/src/java.base/share/classes/java/lang/Integer.java
b/src/java.base/share/classes/java/lang/Integer.java
index 1c5b3c414ba..d50748c369e 100644
--- a/src/java.base/share/classes/java/lang/Integer.java
+++ b/src/java.base/share/classes/java/lang/Integer.java
@@ -28,4 +28,5 @@
import jdk.internal.misc.CDS;
import jdk.internal.misc.VM;
+import jdk.internal.vm.ConstantSupport;
import jdk.internal.vm.annotation.ForceInline;
import jdk.internal.vm.annotation.IntrinsicCandidate;
@@ -416,4 +417,7 @@ private static void formatUnsignedIntUTF16(int val, int
shift, byte[] buf, int l
}
+@Stable
+static final String[] TO_STRINGS = { "-1", "0", "1" };
+
/**
* Returns a {@code String} object representing the
@@ -428,4 +432,8 @@ private static void formatUnsignedIntUTF16(int val, int
shift, byte[] buf, int l
@IntrinsicCandidate
public static String toString(int i) {
+if (ConstantSupport.isCompileConstant(i) &&
+(i >= -1) && (i <= 1)) {
+return TO_STRINGS[i + 1];
+}
int size = stringSize(i);
if (COMPACT_STRINGS) {
diff --git a/test/micro/org/openjdk/bench/java/lang/Integers.java
b/test/micro/org/openjdk/bench/java/lang/Integers.java
index 43ceb5d18d2..28248593a73 100644
--- a/test/micro/org/openjdk/bench/java/lang/Integers.java
+++ b/test/micro/org/openjdk/bench/java/lang/Integers.java
@@ -91,4 +91,18 @@ public void decode(Blackhole bh) {
}
+@Benchmark
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+public String toStringConstYay() {
+return Integer.toString(0);
+}
+
+int v = 0;
+
+@Benchmark
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+public String toStringConstNope() {
+return Integer.toString(v);
+}
+
/** Performs toString on small values, just a couple of digits. */
@Benchmark
Benchmark (size) Mode Cnt Score
Error Units
Integers.toStringConstNope500 avgt 15 3,599 ?
0,034 ns/op
Integers.toStringConstNope:gc.alloc.rate.norm 500 avgt 15 48,000 ?
0,001B/op
Integers.toStringConstNope:gc.time500 avgt 15223,000
ms
Integers.toStringConstYay 500 avgt 15 0,568 ?
0,046 ns/op
Integers.toStringConstYay:gc.alloc.rate.norm 500 avgt 15 ? 10??
B/op
Think about it as simplifying/avoiding the need for full compiler intrinsics. I
could, in principle, do this by intrinsifying `Integer.toString` completely,
check the same `isCon`, and then either construct the access to some String
constant, or arrange the call to actual toString slow path. That would not be
as simple as doing the similar thing in plain Java, with just a little of
compiler support in form of `ConstantSupport`.
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1910449450
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Thu, 25 Jan 2024 12:52:21 GMT, Maurizio Cimadamore
wrote:
>>> > Naive question: the right way to use this would be almost invariably be
>>> > like this:
>>> > ```
>>> > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
>>> > // fast-path
>>> > }
>>> > // slow path
>>> > ```
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > Right?
>>>
>>> Yes, I think so.
>>
>> But then whatever is in the fast path and `fooHasCertainStaticProperties`
>> are never profiled because never executed by the interpreter or c1. So
>> `fooHasCertainStaticProperties` will likely not be inlined and c2 will do a
>> poor (or rather not as good as you'd like) job of compiling whatever is in
>> the fast path.
>
>> > > Naive question: the right way to use this would be almost invariably be
>> > > like this:
>> > > ```
>> > > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
>> > > // fast-path
>> > > }
>> > > // slow path
>> > > ```
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > > Right?
>> >
>> >
>> > Yes, I think so.
>>
>> But then whatever is in the fast path and `fooHasCertainStaticProperties`
>> are never profiled because never executed by the interpreter or c1. So
>> `fooHasCertainStaticProperties` will likely not be inlined and c2 will do a
>> poor (or rather not as good as you'd like) job of compiling whatever is in
>> the fast path.
>
> I suppose perhaps it is implied that `fooHasCertainStaticProperties` should
> have `@ForceInline` ? But yes, there seems to be several assumptions in how
> this logic is supposed to be used, and at the moment, it seems to me more of
> a footgun than something actually useful (but I admit my ignorance on the
> subject).
> @mcimadamore Yes this is hard to use apart from the simple cases. Considering
> we have already used this technique in the `MethodHandle` implementation, I
> think there are valid use cases.
I don't 100% buy the `MethodHandleImpl` analogy. In that case the check is not
simply used to save a branch, but to spare spinning of a completely new lambda
form. That is a very heavy operation. What I'm trying to say is that I'm not
too sure how robust of a mechanism this is in the context of
micro(nano?)-optimizations (such as the one you are considering).
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1910359911
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Thu, 25 Jan 2024 12:52:21 GMT, Maurizio Cimadamore
wrote:
>>> > Naive question: the right way to use this would be almost invariably be
>>> > like this:
>>> > ```
>>> > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
>>> > // fast-path
>>> > }
>>> > // slow path
>>> > ```
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > Right?
>>>
>>> Yes, I think so.
>>
>> But then whatever is in the fast path and `fooHasCertainStaticProperties`
>> are never profiled because never executed by the interpreter or c1. So
>> `fooHasCertainStaticProperties` will likely not be inlined and c2 will do a
>> poor (or rather not as good as you'd like) job of compiling whatever is in
>> the fast path.
>
>> > > Naive question: the right way to use this would be almost invariably be
>> > > like this:
>> > > ```
>> > > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
>> > > // fast-path
>> > > }
>> > > // slow path
>> > > ```
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > > Right?
>> >
>> >
>> > Yes, I think so.
>>
>> But then whatever is in the fast path and `fooHasCertainStaticProperties`
>> are never profiled because never executed by the interpreter or c1. So
>> `fooHasCertainStaticProperties` will likely not be inlined and c2 will do a
>> poor (or rather not as good as you'd like) job of compiling whatever is in
>> the fast path.
>
> I suppose perhaps it is implied that `fooHasCertainStaticProperties` should
> have `@ForceInline` ? But yes, there seems to be several assumptions in how
> this logic is supposed to be used, and at the moment, it seems to me more of
> a footgun than something actually useful (but I admit my ignorance on the
> subject).
@mcimadamore Yes this is hard to use apart from the simple cases. Considering
we have already used this technique in the `MethodHandle` implementation, I
think there are valid use cases.
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1910271891
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Thu, 25 Jan 2024 07:41:27 GMT, Roland Westrelin wrote:
> > > Naive question: the right way to use this would be almost invariably be
> > > like this:
> > > ```
> > > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
> > > // fast-path
> > > }
> > > // slow path
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > Right?
> >
> >
> > Yes, I think so.
>
> But then whatever is in the fast path and `fooHasCertainStaticProperties` are
> never profiled because never executed by the interpreter or c1. So
> `fooHasCertainStaticProperties` will likely not be inlined and c2 will do a
> poor (or rather not as good as you'd like) job of compiling whatever is in
> the fast path.
I suppose perhaps it is implied that `fooHasCertainStaticProperties` should
have `@ForceInline` ? But yes, there seems to be several assumptions in how
this logic is supposed to be used, and at the moment, it seems to me more of a
footgun than something actually useful (but I admit my ignorance on the
subject).
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1910140980
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Wed, 24 Jan 2024 18:56:15 GMT, Aleksey Shipilev wrote:
> > Naive question: the right way to use this would be almost invariably be
> > like this:
> > ```
> > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
> > // fast-path
> > }
> > // slow path
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > Right?
>
> Yes, I think so.
But then whatever is in the fast path and `fooHasCertainStaticProperties` are
never profiled because never executed by the interpreter or c1. So
`fooHasCertainStaticProperties` will likely not be inlined and c2 will do a
poor (or rather not as good as you'd like) job of compiling whatever is in the
fast path.
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1909531426
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Wed, 24 Jan 2024 18:56:15 GMT, Aleksey Shipilev wrote:
>>> Naive question: the right way to use this would be almost invariably be
>>> like this:
>>>
>>> ```
>>> if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
>>> // fast-path
>>> }
>>> // slow path
>>> ```
>>>
>>> Right? Then the expectation is that during interpreter and C1,
>>> `isCompileConstant` always returns false, so we just never take the fast
>>> path (but we probably still pay for the branch, right?). And, when we get
>>> to C2 and this method is inlined, at this point we know that either `foo`
>>> is constant or not. If it is constant we can check other conditions on foo
>>> (which presumably is cheap because `foo` is constant) and maybe take the
>>> fast-path. In both cases, there's no branch in the generated code because
>>> we know "statically" when inlining if `foo` has the right shape or not.
>>> Correct?
>>
>> P.S. if this is correct, please consider adding something along those lines
>> in the javadoc of `isCompileConstant`; as it stands it is a bit obscure to
>> understand how this thing might be used, and what are the common pitfalls to
>> avoid when using it.
>
>> Naive question: the right way to use this would be almost invariably be like
>> this:
>>
>> ```
>> if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
>> // fast-path
>> }
>> // slow path
>> ```
>>
>> Right?
>
> Yes, I think so.
>
>> Then the expectation is that during interpreter and C1, `isCompileConstant`
>> always returns false, so we just never take the fast path (but we probably
>> still pay for the branch, right?).
>
> Yes, I think so. For C1, we would still prune the "dead" path, because C1 is
> able to know that `if (false)` is never taken. We do pay with the branch and
> the method call in interpreter. (There are ways to special-case these
> intrinsics for interpreter too, if we choose to care.)
>
>> And, when we get to C2 and this method is inlined, at this point we know
>> that either `foo` is constant or not. If it is constant we can check other
>> conditions on foo (which presumably is cheap because `foo` is constant) and
>> maybe take the fast-path. In both cases, there's no branch in the generated
>> code because we know "statically" when inlining if `foo` has the right shape
>> or not. Correct?
>
> Yes. I think the major use would be using `constexpr`-like code on "const"
> path, so that the entire code constant-folds completely, _or_ just compiles
> to branch-less "generic" version. In [my
> experiments](https://github.com/openjdk/jdk/pull/17527#issuecomment-1906379544)
> with `Integer.toString` that certainly happens. But that is not a
> requirement, and we could probably still reap some benefits from partial
> constant folds; but at that point we would need to prove that a "partially
> const" path is better than generic "non-const" path under the same conditions.
>
> I agree it would be convenient to put some examples in javadoc. @merykitty, I
> can help you with that, if you want.
@shipilev I can come up with 2 examples that are pretty generic:
void checkIndex(int index, int length) {
boolean indexPositive = index >= 0;
if (ConstantSupport.isCompileConstant(indexPositive) && indexPositive) {
if (index >= length) {
throw;
}
return;
}
if (length < 0 || Integer.compareUnsigned(index, length) >= 0) {
throw;
}
}
bool equals(Point p1, Point p2) {
idEqual = p1 == p2;
if (ConstantSupport.isCompileConstant(idEqual) && idEqual) {
return true;
}
return p1.x == p2.x && p1.y == p2.y;
}
@mcimadamore Yes I believe your expectations are correct. Pitfalls may vary
case-by-case, but I just realised that since we do not have profile information
in the fast path, the compiler may be less willingly to inline the callees
here. While it has not been an issue, a solution I can think of is to have
something like `ConstantSupport::evaluate` in which the compiler will try to
inline infinitely expecting constant-folding similar to how a `constexpr`
variable behaves in C++ (and maybe bail-out compilation if the final result is
not a constant, too).
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1909272480
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Wed, 24 Jan 2024 18:51:27 GMT, Maurizio Cimadamore
wrote:
> Naive question: the right way to use this would be almost invariably be like
> this:
>
> ```
> if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
> // fast-path
> }
> // slow path
> ```
>
> Right?
Yes, I think so.
> Then the expectation is that during interpreter and C1, `isCompileConstant`
> always returns false, so we just never take the fast path (but we probably
> still pay for the branch, right?).
Yes, I think so. For C1, we would still prune the "dead" path, because C1 is
able to know that `if (false)` is never taken. We do pay with the branch and
the method call in interpreter. (There are ways to special-case these
intrinsics for interpreter too, if we choose to care.)
> And, when we get to C2 and this method is inlined, at this point we know that
> either `foo` is constant or not. If it is constant we can check other
> conditions on foo (which presumably is cheap because `foo` is constant) and
> maybe take the fast-path. In both cases, there's no branch in the generated
> code because we know "statically" when inlining if `foo` has the right shape
> or not. Correct?
Yes. I think the major use would be using `constexpr`-like code on "const"
path, so that the entire "const" branch constant-folds completely. In [my
experiments](https://github.com/openjdk/jdk/pull/17527#issuecomment-1906379544)
with `Integer.toString` that certainly happens. But that is not a requirement,
and we could probably still reap some benefits from partial constant folds; but
at that point we would need to prove that a "partially const" path is better
than generic "non-const" path under the same conditions.
I agree it would be convenient to put some examples in javadoc.
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1908736651
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Wed, 24 Jan 2024 18:48:03 GMT, Maurizio Cimadamore
wrote:
> Naive question: the right way to use this would be almost invariably be like
> this:
>
> ```
> if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
> // fast-path
> }
> // slow path
> ```
>
> Right? Then the expectation is that during interpreter and C1,
> `isCompileConstant` always returns false, so we just never take the fast path
> (but we probably still pay for the branch, right?). And, when we get to C2
> and this method is inlined, at this point we know that either `foo` is
> constant or not. If it is constant we can check other conditions on foo
> (which presumably is cheap because `foo` is constant) and maybe take the
> fast-path. In both cases, there's no branch in the generated code because we
> know "statically" when inlining if `foo` has the right shape or not. Correct?
P.S. if this is correct, please consider adding something along those lines in
the javadoc of `isCompileConstant`; as it stands it is a bit obscure to
understand how this thing might be used, and what are the common pitfalls to
avoid when using it.
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1908729766
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Wed, 24 Jan 2024 10:33:05 GMT, Quan Anh Mai 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: > > address reviews src/java.base/share/classes/jdk/internal/vm/ConstantSupport.java line 32: > 30: /** > 31: * Just-in-time-compiler-related queries > 32: */ This looks like a stale comment. - PR Review Comment: https://git.openjdk.org/jdk/pull/17527#discussion_r1465397036
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
On Wed, 24 Jan 2024 10:33:05 GMT, Quan Anh Mai 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:
>
> address reviews
Naive question: the right way to use this would be almost invariably be like
this:
if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) {
// fast-path
}
// slow path
Right? Then the expectation is that during interpreter and C1,
`isCompileConstant` always returns false, so we just never take the fast path
(but we probably still pay for the branch, right?). And, when we get to C2 and
this method is inlined, at this point we know that either `foo` is constant or
not. If it is constant we can check other conditions on foo (which presumably
is cheap because `foo` is constant) and maybe take the fast-path. In both
cases, there's no branch in the generated code because we know "statically"
when inlining if `foo` has the right shape or not. Correct?
-
PR Comment: https://git.openjdk.org/jdk/pull/17527#issuecomment-1908724632
Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]
> 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: address reviews - Changes: - all: https://git.openjdk.org/jdk/pull/17527/files - new: https://git.openjdk.org/jdk/pull/17527/files/3ecb2c66..b4445e2e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17527&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17527&range=04-05 Stats: 36 lines in 4 files changed: 10 ins; 0 del; 26 mod Patch: https://git.openjdk.org/jdk/pull/17527.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17527/head:pull/17527 PR: https://git.openjdk.org/jdk/pull/17527
