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