On Thu, 23 May 2024 22:06:38 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:

>> Scott Gibbons has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Addressing review comments
>
> src/hotspot/cpu/x86/stubGenerator_x86_64.hpp line 582:
> 
>> 580: 
>> 581: #ifdef COMPILER2
>> 582:   void generate_string_indexof_stubs(address *fnptrs, 
>> StrIntrinsicNode::ArgEncoding ae);
> 
> Is it possible to make `generate_string_indexof_stubs()` as local static 
> method in `stubGenerator_x86_64_string.cpp` and pass `StubGenerator*` as 
> argument?
> Then you don't to include "opto/intrinsicnode.hpp" here.

Done.

> src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2023, Intel Corporation. All rights reserved.
> 
> 2024 year

Fixed.

> src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 27:
> 
>> 25: 
>> 26: #include "precompiled.hpp"
>> 27: #ifdef COMPILER2
> 
> You can exclude this file completely from compilation without this `#ifdef` 
> if you prefix the name with `c2_`.
> There is code in make files to exclude such files: 
> [JvmFeatures.gmk#L38](https://github.com/openjdk/jdk/blob/master/make/hotspot/lib/JvmFeatures.gmk#L38)

I will change the name and remove the #ifdef.  Thanks for this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612401461
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612399243
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612400071

Reply via email to