RFR: 8304034: Remove redundant and meaningless comments in opto [v2]

Vladimir Kozlov kvn at openjdk.org
Mon Mar 13 16:38:13 UTC 2023


On Mon, 13 Mar 2023 07:42:59 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> Please help review this trivial change to remove redundant and meaningless comments in `hotspot/share/opto` directory.
>> 
>> They are either
>> 1. Repeat the function name that the function they comment for.
>> 2. Makes no sense, e.g. `//----Idealize----` 
>> 
>> And I think original CC-style code (`if( test )`,`call( arg )`) can be formatted in one go, instead of formatting the near code when someone touches them. But this may form a big patch, and it confuses code blame, so I left this work until we reach a consensus.
>> 
>> Thanks!
>
> Yi Yang has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   8304034: Remove redundant and meaningless comments in opto

I agree that comments which duplicate method names are useless. But some comment have meaning.
Consider cleanup multiply sequential empty lines too.

src/hotspot/share/opto/loopTransform.cpp line 61:

> 59: 
> 60: 
> 61: 

May be look to remove multiply sequential empty lines too. One empty line as code separator is enough.

src/hotspot/share/opto/loopopts.cpp line 1840:

> 1838: //                   C L O N E   A   L O O P   B O D Y
> 1839: //
> 1840: 

This can be removed too. We have this comment before `clone_loop()` method.

src/hotspot/share/opto/macro.cpp line 1134:

> 1132: }
> 1133: 
> 1134: //=============================================================================

I would leave this one

src/hotspot/share/opto/macro.cpp line 1173:

> 1171: // oop flavor.
> 1172: //
> 1173: //=============================================================================

this one

src/hotspot/share/opto/macro.cpp line 1179:

> 1177: // trigger exceptions go the slow route.  Also, it must be small enough so
> 1178: // that heap_top + size_in_bytes does not wrap around the 4Gig limit.
> 1179: //=============================================================================j//

and this one.

src/hotspot/share/opto/parse1.cpp line 101:

> 99: #endif
> 100: 
> 101: //------------------------------ON STACK REPLACEMENT---------------------------

Leave this.

src/hotspot/share/opto/phaseX.cpp line 46:

> 44: //=============================================================================
> 45: #define NODE_HASH_MINIMUM_SIZE    255
> 46: //------------------------------NodeHash---------------------------------------

Add empty line to separate `#define` from following code.

src/hotspot/share/opto/phaseX.cpp line 1925:

> 1923: uint PhaseCCP::_total_constants = 0;
> 1924: #endif
> 1925: //------------------------------PhaseCCP---------------------------------------

Add empty line.

src/hotspot/share/opto/regmask.hpp line 35:

> 33: class LRG;
> 34: 
> 35: //-------------Non-zero bit search methods used by RegMask---------------------

Leave this.

src/hotspot/share/opto/runtime.cpp line 195:

> 193: //=============================================================================
> 194: // Opto compiler runtime routines
> 195: //=============================================================================

Leave these.

src/hotspot/share/opto/runtime.cpp line 706:

> 704: }
> 705: 
> 706: //-------------- currentTimeMillis, currentTimeNanos, etc

Leave this.

src/hotspot/share/opto/runtime.cpp line 1309:

> 1307: }
> 1308: 
> 1309: //------------- Interpreter state access for on stack replacement

Leave this.

src/hotspot/share/opto/superword.cpp line 46:

> 44: //
> 45: //                  S U P E R W O R D   T R A N S F O R M
> 46: //=============================================================================

Leave this.

src/hotspot/share/opto/vectornode.hpp line 133:

> 131: };
> 132: 
> 133: //===========================Vector=ALU=Operations=============================

Leave it.

src/hotspot/share/opto/vectornode.hpp line 825:

> 823: };
> 824: 
> 825: //================================= M E M O R Y ===============================

Leave it.

src/hotspot/share/opto/vectornode.hpp line 1114:

> 1112: };
> 1113: 
> 1114: //=========================Promote_Scalar_to_Vector============================

Leave it

src/hotspot/share/opto/vectornode.hpp line 1171:

> 1169: };
> 1170: 
> 1171: //========================Pack_Scalars_into_a_Vector===========================

Leave it

src/hotspot/share/opto/vectornode.hpp line 1267:

> 1265: };
> 1266: 
> 1267: //========================Extract_Scalar_from_Vector===========================

Leave it.

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

PR: https://git.openjdk.org/jdk/pull/12995


More information about the hotspot-compiler-dev mailing list