RFR: 8353290: C2: Refactor PhaseIdealLoop::is_counted_loop() [v2]

[Kangcheng Xu]

On Sat, 12 Apr 2025 04:07:42 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   reviewer suggested changes
>
> src/hotspot/share/opto/loopnode.hpp line 267:
> 
>> 265:     const TypeInteger* trunc_type = nullptr;
>> 266:   };
>> 267:   static TruncatedIncrement match_incr_with_optional_truncation(Node* expr, BasicType bt);
> 
> Just a drive-by comment: this function and some others that you created are returning a value by copy, for performance reasons it may be better to return a reference or even a pointer as usually is the case in HotSpot.

I'm not sure if this is always the case but please correct me if I'm wrong: a cpp compiler already optimizes these kinds of patterns and structs are only allocated on the caller stack frame with a pointer to it passed into callee as an argument (or even leaving fields in registers if the struct is small enough). No copying is actually happening.

I agree this *syntactically looks costly*, please let me know if you find it necessary to switch to passing in a struct reference as an argument.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24458#discussion_r2047284718