RFR: 8320649: C2: Optimize scoped values [v4]
Roland Westrelin
roland at openjdk.org
Tue Jan 30 12:16:30 UTC 2024
On Thu, 18 Jan 2024 08:53:18 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
>>
>> merge fix
>
> src/hotspot/share/opto/loopPredicate.cpp line 1648:
>
>> 1646: tty->print("Predicate invariant if: %d ", new_predicate_iff->_idx);
>> 1647: loop->dump_head();
>> 1648: } else if (TraceLoopOpts) {
>
> Why not have them as separate ifs? What if someone enables both, will they not miss a line?
That's the code pattern used elsewhere in `PhaseIdealLoop::loop_predication_impl_helper()`.
> src/hotspot/share/opto/loopnode.cpp line 4717:
>
>> 4715: assert(!_igvn.delay_transform(), "");
>> 4716: _igvn.set_delay_transform(true);
>> 4717: for (uint i = _scoped_value_get_nodes.size(); i > 0; i--) {
>
> Suggestion:
>
> for (uint i = _scoped_value_get_nodes.size()-1; i >= 0; i--) {
An unsigned value is always `>= 0`. Wouldn't your suggestion turn the loop into an infinite loop?
> src/hotspot/share/opto/subnode.hpp line 340:
>
>> 338:
>> 339: Node* mem() const {
>> 340: return in(Memory);
>
> Why not verify that this is a `MemNode`?
MemNodes are not the only ones carrying memory state (projection for a call, membar etc).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1471104465
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1471106908
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1470836111
More information about the hotspot-compiler-dev
mailing list