On Mon, 2 Feb 2026 10:29:20 GMT, Tobias Hartmann <[email protected]> wrote:
>> Damon Fenacci has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> JDK-8374582: revert wrong copyright change
>
> src/hotspot/share/opto/library_call.cpp line 894:
>
>> 892:
>> 893: inline Node* LibraryCallKit::generate_negative_guard(Node* index,
>> RegionNode* region,
>> 894: Node** pos_index,
>> bool is_opaque) {
>
> As we discussed offline, I think `with_opaque` is better here.
Renamed. Thanks @TobiHartmann.
> src/hotspot/share/opto/opaquenode.hpp line 148:
>
>> 146: class OpaqueConstantBoolNode : public Node {
>> 147: private:
>> 148: bool _constant;
>
> Should this be `const`?
Yep, fixed.
> src/hotspot/share/opto/opaquenode.hpp line 150:
>
>> 148: bool _constant;
>> 149: public:
>> 150: OpaqueConstantBoolNode(Compile* C, Node* tst, bool constant) :
>> Node(nullptr, tst), _constant(constant) {
>
> An alternative would be to have the `constant` be an actual input node
> instead of a field. In macro expansion, you could then do
> `_igvn.replace_node(n, n->in(2));` instead (maybe define an enum for the
> input indices). I don't have a strong opinion on this though and leave it up
> to you to decide 🙂
Cool trick! 😃... but now I can't decide between the two 😆 @chhagedorn do you
fancy being the tiebreaker?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2754030906
PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2754030537
PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2754032138
