RFR: 8374582: [REDO] Move input validation checks to Java for java.lang.StringCoding intrinsics [v3]

[Damon Fenacci]

On Wed, 28 Jan 2026 08:13:23 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Damon Fenacci has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8374852: fix star layout
>>   
>>   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>
> src/hotspot/share/opto/opaquenode.hpp line 146:
> 
>> 144: // builds, we keep the actual checks as additional verification code (i.e. removing OpaqueCheckNodes and use the
>> 145: // BoolNode inputs instead).
>> 146: class OpaqueCheckNode : public Node {
> 
> I've also thought about the name. `OpaqueCheck` is already a good indication what the node is about. Maybe we could go a step further and call it `OpaqueConstantBoolNode` to emphasize more that it is belonging to a `BoolNode` whose result we already know. What do you think?
> 
> Then we could also think about changing `_positive` to `_constant` (still can be a boolean to just pass true and false which seems more intuitive then passing in 1 and 0).

I was still had a doubt about what to put first (`Constant` or `Bool`) but I think `ConstantBool` is actually more correct.

I suppose `_constant` is better than `_value` since we use it already 😉 
Done.

> src/hotspot/share/opto/opaquenode.hpp line 148:
> 
>> 146: class OpaqueCheckNode : public Node {
>> 147:  private:
>> 148:   bool _positive;
> 
> Now that we define a field, we also need to override `size_of()` (see for example `OpaqueMultiversioningNode`).

Good to know. Thanks!

> src/hotspot/share/opto/opaquenode.hpp line 150:
> 
>> 148:   bool _positive;
>> 149:  public:
>> 150:   OpaqueCheckNode(Compile* C, Node* tst, bool positive) : Node(nullptr, tst), _positive(positive) {
> 
> `tst` is probably almost always a `BoolNode`. I'm wondering if it could also be a constant because we already folded the `BoolNode`? But then it's probably also useless to create the opaque node in the first place.

Hmmm... I find it hard to totally exclude a constant (e.g. if its inputs are constant...?). In that case we could skip all the opaque business (I guess in the few places where new `OpaqueConstantBool` nodes are created). On the other hand the opaque node should only really delay the folding... 🤔

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2737356877
PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2737353309
PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2737355777