RFR: 8299974: Replace NULL with nullptr in share/adlc/
Johan Sjölen
jsjolen at openjdk.org
Wed May 24 18:01:28 UTC 2023
On Tue, 16 May 2023 11:54:20 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> Hi, this PR changes all occurrences of NULL to nullptr for the subdirectory share/adlc. Unfortunately the script that does the change isn't perfect, and so we need to comb through these manually to make sure nothing has gone wrong. I also review these changes but things slip past my eyes sometimes.
>
> Here are some typical things to look out for:
>
> 1. No changes but copyright header changed (probably because I reverted some changes but forgot the copyright).
> 2. Macros having their NULL changed to nullptr, these are added to the script when I find them. They should be NULL.
> 3. nullptr in comments and logs. We try to use lower case "null" in these cases as it reads better. An exception is made when code expressions are in a comment.
>
> An example of this:
>
> ```c++
> // This function returns null
> void* ret_null();
> // This function returns true if *x == nullptr
> bool is_nullptr(void** x);
>
>
> Note how `nullptr` participates in a code expression here, we really are talking about the specific value `nullptr`.
>
> Thanks!
Mostly alignment and comment issues.
Found all (most?) of the faulty NULL -> null conversions.
Hm. I think that this is completely broken. This is a compiler/code generation tool so any string containing `null` instead of `NULL` now is incorrect. I'll leave this until a bit later.
Builds on linux-x64, running tier1. Open for review.
src/hotspot/share/adlc/adlArena.cpp line 56:
> 54:
> 55: AdlChunk::AdlChunk(size_t length) {
> 56: _next = nullptr; // Chain on the linked list
Align
src/hotspot/share/adlc/adlArena.cpp line 168:
> 166: AdlArena *a = new AdlArena(this); // New empty arena
> 167: _first = _chunk = nullptr; // Normal, new-arena initialization
> 168: _hwm = _max = nullptr;
Align
src/hotspot/share/adlc/adlparse.cpp line 871:
> 869: do {
> 870: char *pType = nullptr; // parameter type
> 871: char *pName = nullptr; // parameter name
Align?
src/hotspot/share/adlc/adlparse.cpp line 973:
> 971: void ADLParser::frame_parse(void) {
> 972: FrameForm *frame; // Information about stack-frame layout
> 973: char *desc = nullptr; // String representation of frame
align
src/hotspot/share/adlc/adlparse.cpp line 1067:
> 1065: }
> 1066: // !!!!! !!!!!
> 1067: // if(frame->_interpreter_frame_ptr_reg == null) {
nullptr
src/hotspot/share/adlc/adlparse.cpp line 1219:
> 1217: //------------------------------return_value_parse-----------------------------
> 1218: char *ADLParser::return_value_parse() {
> 1219: char *desc = nullptr; // String representation of return_value
align
src/hotspot/share/adlc/adlparse.cpp line 2033:
> 2031: void ADLParser::peep_parse(void) {
> 2032: Peephole *peep; // Pointer to current peephole rule form
> 2033: char *desc = nullptr; // String representation of rule
align
src/hotspot/share/adlc/adlparse.cpp line 3159:
> 3157: // if ( _curchar != ',' && _curchar != ')' ) {
> 3158: // parse_err(SYNERR, "expected ',' or ')' after encode method inside ins_encode.\n");
> 3159: // return null;
nullptr
src/hotspot/share/adlc/adlparse.cpp line 3837:
> 3835: MatchRule *ADLParser::match_parse(FormDict &operands) {
> 3836: MatchRule *match; // Match Rule class for instruction/operand
> 3837: char *cnstr = nullptr; // Code for constructor
align
src/hotspot/share/adlc/adlparse.cpp line 3847:
> 3845: skipws(); // Skip whitespace
> 3846: if ( _curchar == ';' ) { // Semicolon is valid terminator
> 3847: cnstr = nullptr; // no constructor for this form
align
src/hotspot/share/adlc/adlparse.cpp line 3853:
> 3851: parse_err(SYNERR, "invalid construction of match rule\n"
> 3852: "Missing ';' or invalid '%%{' and '%%}' constructor\n");
> 3853: return nullptr; // No MatchRule to return
align
src/hotspot/share/adlc/adlparse.cpp line 3871:
> 3869: skipws(); // Skip whitespace
> 3870: if ( _curchar == ';' ) { // Semicolon is valid terminator
> 3871: desc = nullptr; // no constructor for this form
align
src/hotspot/share/adlc/adlparse.cpp line 4304:
> 4302: Attribute *ADLParser::attr_parse(char* ident) {
> 4303: Attribute *attrib; // Attribute class
> 4304: char *cost = nullptr; // String representation of cost attribute
align
src/hotspot/share/adlc/adlparse.cpp line 4362:
> 4360: // Lookup the root value in the operands dict to perform substitution
> 4361: const char *result = nullptr; // Result type will be filled in later
> 4362: const char *name = token; // local name associated with this node
align
src/hotspot/share/adlc/adlparse.cpp line 4457:
> 4455: char* ADLParser::find_cpp_block(const char* description) {
> 4456: char *next; // Pointer for finding block delimiters
> 4457: char* cppBlock = nullptr; // Beginning of C++ code block
align
src/hotspot/share/adlc/adlparse.cpp line 4794:
> 4792: int result; // Storage for integer result
> 4793:
> 4794: if( _curline == nullptr ) // Return null at EOF.
align
src/hotspot/share/adlc/adlparse.cpp line 4830:
> 4828:
> 4829: if( _curline == nullptr ) // Return null at EOF.
> 4830: return nullptr;
align
src/hotspot/share/adlc/adlparse.cpp line 5188:
> 5186: }
> 5187: }
> 5188: while(_curline != nullptr) { // Check for end of file
align
src/hotspot/share/adlc/adlparse.cpp line 5202:
> 5200: if (*_ptr == '\n') { // keep proper track of new lines
> 5201: next_line(); // skip newlines within comments
> 5202: if (_curline == nullptr) { // check for end of file
align
src/hotspot/share/adlc/adlparse.cpp line 5241:
> 5239: else { ++_ptr; ++next; }
> 5240: }
> 5241: if( _curline != nullptr ) // at end of file _curchar isn't valid
align
src/hotspot/share/adlc/archDesc.cpp line 394:
> 392: // const Form *form = operands[_result];
> 393: // OpClassForm *opcForm = form ? form->is_opclass() : null;
> 394: // assert(opcForm != null, "Match Rule contains invalid operand name.");
nullptr
src/hotspot/share/adlc/dfa.cpp line 196:
> 194: // If unpredicated vector unary operation, add one extra check, i.e. right
> 195: // child should be null, to distinguish from the predicated version.
> 196: fprintf(fp, " && _kids[1] == null");
nullptr
src/hotspot/share/adlc/dfa.cpp line 952:
> 950: printf("%s", (_result == null ? "null" : _result ) );
> 951: printf("%s", (_constraint == null ? "null" : _constraint ) );
> 952: printf("%s", (_valid == null ? "null" : _valid ) );
nullptr
src/hotspot/share/adlc/dict2.cpp line 207:
> 205: b->_keyvals[b->_cnt+b->_cnt+1] = val;
> 206: b->_cnt++;
> 207: return nullptr; // Nothing found prior
align
src/hotspot/share/adlc/forms.cpp line 306:
> 304: // Form *cur = _root;
> 305: // Form *next = null;
> 306: // for( ; (cur = next) != null; ) {
nullptr
src/hotspot/share/adlc/forms.hpp line 517:
> 515: Max = 0x7fffffff
> 516: };
> 517: const char *_external_name; // if !null, then print this instead of _expr
if not
src/hotspot/share/adlc/formssel.cpp line 301:
> 299: if (strcmp(opType,"ThreadLocal")==0) {
> 300: fprintf(stderr, "Warning: ThreadLocal instruction %s should be named 'tlsLoadP_*'\n",
> 301: (_ident == nullptr ? "null" : _ident));
nullptr
src/hotspot/share/adlc/formssel.cpp line 714:
> 712: // // unique def, some uses
> 713: // // must return bottom unless all uses match def
> 714: // unique = null;
nullptr?
src/hotspot/share/adlc/formssel.cpp line 984:
> 982: } else {
> 983: // This would be a nice warning but it triggers in a few places in a benign way
> 984: // if (_matrule != null && !expands()) {
nullptr
src/hotspot/share/adlc/formssel.cpp line 2947:
> 2945: // // This list may not own its elements if copied via assignment
> 2946: // Component *component;
> 2947: // for (reset(); (component = iter()) != null;) {
nullptr
src/hotspot/share/adlc/formssel.cpp line 3385:
> 3383: : mnode->_rChild->_opType;
> 3384: }
> 3385: // Else, May be simple chain rule: (Set dst operand_form), rightStr=null;
nullptr
src/hotspot/share/adlc/output_c.cpp line 779:
> 777: fprintf(fp_cpp, "static const Pipeline pipeline_class_Zero_Instructions(0, 0, true, 0, 0, false, false, false, false, null, null, null, Pipeline_Use(0, 0, 0, null));\n\n");
> 778: fprintf(fp_cpp, "static const Pipeline pipeline_class_Unknown_Instructions(0, 0, true, 0, 0, false, true, true, false, null, null, null, Pipeline_Use(0, 0, 0, null));\n\n");
> 779:
nullptr
src/hotspot/share/adlc/output_c.cpp line 892:
> 890: }
> 891: else
> 892: fprintf(fp_cpp, " null,");
nullptr
src/hotspot/share/adlc/output_c.cpp line 903:
> 901: pipeline_res_mask_index+1);
> 902: else
> 903: fprintf(fp_cpp, "null");
nullptr
src/hotspot/share/adlc/output_c.cpp line 1050:
> 1048: print_block_index(fp, inst_position);
> 1049: fprintf(fp, ");\n inst%d = (n->is_Mach()) ? ", inst_position);
> 1050: fprintf(fp, "n->as_Mach() : null;\n }\n");
nullptr
src/hotspot/share/adlc/output_c.cpp line 1056:
> 1054: // Test we have the correct instruction by comparing the rule.
> 1055: if( parent != -1 ) {
> 1056: fprintf(fp, " matches = matches && (inst%d != null) && (inst%d->rule() == %s_rule);\n",
nullptr
src/hotspot/share/adlc/output_c.cpp line 1363:
> 1361: for (int i = 0; i <= max_position; i++) {
> 1362: fprintf(fp, " inst%d->set_removed();\n", i);
> 1363: fprintf(fp, " cfg_->map_node_to_block(inst%d, null);\n", i);
nullptr
src/hotspot/share/adlc/output_c.cpp line 1388:
> 1386: // ...
> 1387: // MachNode *instMAX = null;
> 1388: //
nullptr
src/hotspot/share/adlc/output_c.cpp line 1403:
> 1401: fprintf(fp, " MachNode *inst0 = this;\n");
> 1402: } else {
> 1403: fprintf(fp, " MachNode *inst%d = null;\n", i);
nullptr
src/hotspot/share/adlc/output_c.cpp line 1524:
> 1522: }
> 1523: else {
> 1524: fprintf(fp," MachNode *tmp%d = null;\n", i);
nullptr
src/hotspot/share/adlc/output_c.cpp line 1554:
> 1552:
> 1553: // Declare variable to hold root of expansion
> 1554: fprintf(fp," MachNode *result = null;\n");
nullptr
src/hotspot/share/adlc/output_c.cpp line 1642:
> 1640: cnt, new_pos, exp_pos-node->num_opnds(), opid);
> 1641: // Check for who defines this operand & add edge if needed
> 1642: fprintf(fp," if(tmp%d != null)\n", exp_pos);
nullptr
src/hotspot/share/adlc/output_c.cpp line 2862:
> 2860: fprintf(fp," }\n");
> 2861: fprintf(fp," ShouldNotReachHere();\n");
> 2862: fprintf(fp," return null;\n");
nullptr
src/hotspot/share/adlc/output_c.cpp line 3354:
> 3352: fprintf(_CPP_PIPELINE_file._fp, "const Pipeline * %*s::pipeline_class() { return %s; }\n",
> 3353: max_ident_len, "MachNode", _pipeline ? "(&pipeline_class_Unknown_Instructions)" : "null");
> 3354: fprintf(_CPP_PIPELINE_file._fp, "const Pipeline * %*s::pipeline() const { return pipeline_class(); }\n",
nullptr
src/hotspot/share/adlc/output_c.cpp line 3903:
> 3901: // Generate the case statement for this opcode
> 3902: fprintf(fp_cpp, " case %s:", opEnumName);
> 3903: fprintf(fp_cpp, " return null;\n");
nullptr
src/hotspot/share/adlc/output_c.cpp line 3915:
> 3913:
> 3914: // Generate the closing for method Matcher::MachOperGenerator
> 3915: fprintf(fp_cpp, " return null;\n");
nullptr
src/hotspot/share/adlc/output_c.cpp line 4174:
> 4172:
> 4173: // Generate the closing for method Matcher::MachNodeGenerator
> 4174: fprintf(fp_cpp, " return null;\n");
nullptr
src/hotspot/share/adlc/output_h.cpp line 583:
> 581: oper.ext_format(fp, globals, 0);
> 582: }
> 583: } else { // oper._format == null
is null
src/hotspot/share/adlc/output_h.cpp line 715:
> 713: else if( inst.is_ideal_mem() ) {
> 714: // Print out the field name if available to improve readability
> 715: fprintf(fp, " if (ra->C->alias_type(adr_type())->field() != null) {\n");
nullptr
src/hotspot/share/adlc/output_h.cpp line 1115:
> 1113:
> 1114: // const char *classname;
> 1115: // for (_pipeline->_classlist.reset(); (classname = _pipeline->_classlist.iter()) != null; ) {
nullptr
-------------
PR Review: https://git.openjdk.org/jdk/pull/14008#pullrequestreview-1428402551
PR Review: https://git.openjdk.org/jdk/pull/14008#pullrequestreview-1442331985
PR Comment: https://git.openjdk.org/jdk/pull/14008#issuecomment-1549688257
PR Comment: https://git.openjdk.org/jdk/pull/14008#issuecomment-1561699876
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195049718
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195049916
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195050812
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195051007
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195051165
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195051376
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195051690
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195052827
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195053371
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195053476
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195053564
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195053647
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195054014
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195054110
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195054282
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195054859
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195055035
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195055284
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195055381
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195055471
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195055895
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204483422
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204484056
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195056915
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195057492
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195057741
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204485353
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195059076
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195059369
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195060351
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195060588
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204488502
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204488791
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204488867
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204489279
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204489338
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204489705
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195061423
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204489843
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204490104
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204490222
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204490341
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204491334
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204492272
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204492782
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204492858
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204493048
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195062510
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1204493636
PR Review Comment: https://git.openjdk.org/jdk/pull/14008#discussion_r1195062686
More information about the hotspot-compiler-dev
mailing list