On Wed, 24 Jan 2024 18:56:15 GMT, Aleksey Shipilev <sh...@openjdk.org> 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
