RFR: JDK-8301074: Replace NULL with nullptr in share/opto/ [v5]
Jesper Wilhelmsson
jwilhelm at openjdk.org
Tue Mar 7 18:48:20 UTC 2023
On Tue, 7 Mar 2023 08:39:49 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
>> Hi, this PR changes all occurrences of NULL to nullptr for the subdirectory share/opto/. 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:
>>
>> No changes but copyright header changed (probably because I reverted some changes but forgot the copyright).
>> Macros having their NULL changed to nullptr, these are added to the script when I find them. They should be NULL.
>> 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:
>>
>>
>> // 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!
>
> Johan Sjölen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
>
> - Merge remote-tracking branch 'origin/JDK-8301074' into JDK-8301074
> - Explicitly use 0 for null in ARM interpreter
> - Merge remote-tracking branch 'origin/master' into JDK-8301074
> - Remove trailing whitespace
> - Check for null string explicitly
> - vkozlov fixes
> - Manual review fixes
> - Fix
> - Fix compile errors
> - Replace NULL with nullptr in share/opto/
Looks good in general. There are a bunch of places where the code previously was intentionally aligned (comments, braces, instructions etc) where it's now not aligned anymore since nullptr has more characters than NULL. I have put notes on the places I noticed.
There are several strings that are modified, some seems to be part of logging and other text that is observable by a user. I'm not sure what our policy is around keeping logging / hs_err.log etc stable so just want to bring it up so that it's not a surprise to anyone.
src/hotspot/share/opto/callGenerator.hpp line 48:
> 46:
> 47: virtual bool do_late_inline_check(Compile* C, JVMState* jvms) { ShouldNotReachHere(); return false; }
> 48: virtual CallGenerator* inline_cg() const { ShouldNotReachHere(); return nullptr; }
'NULL' was shorter than 'false' so it had two spaces before the }. That's not the case anymore so remove a space?
src/hotspot/share/opto/callnode.cpp line 837:
> 835: const TypeInstPtr* inst_t = phase->type(proj)->isa_instptr();
> 836: if ((inst_t != nullptr) && (!inst_t->klass_is_exact() ||
> 837: (inst_t->instance_klass() == boxing_klass))) {
Indentation of this line should increase as the starting parenthesis above has moved.
src/hotspot/share/opto/cfgnode.cpp line 627:
> 625:
> 626: if( cnt <= 1 ) { // Only 1 path in?
> 627: set_req(0, nullptr); // Null control input for region copy
Align comments?
src/hotspot/share/opto/cfgnode.cpp line 1778:
> 1776: return nullptr; // Bail out on funny non-value stuff
> 1777: if( phi->req() <= 3 ) // Need at least 2 matched inputs and a
> 1778: return nullptr; // third unequal input to be worth doing
Align comments.
src/hotspot/share/opto/chaitin.cpp line 382:
> 380: {
> 381: Compile::TracePhase tp("computeLive", &timers[_t_computeLive]);
> 382: _live = nullptr; // Mark live as being not available
Align comment
src/hotspot/share/opto/compile.cpp line 301:
> 299:
> 300: // Initialize worklist
> 301: if (root() != nullptr) { useful.push(root()); }
Looks like the braces was aligned before.
src/hotspot/share/opto/compile.cpp line 1617:
> 1615:
> 1616: // Handle special cases.
> 1617: if (adr_type == nullptr) return alias_type(AliasIdxTop);
Align returns.
src/hotspot/share/opto/compile.cpp line 1760:
> 1758: bool Compile::must_alias(const TypePtr* adr_type, int alias_idx) {
> 1759: if (alias_idx == AliasIdxBot) return true; // the universal category
> 1760: if (adr_type == nullptr) return true; // null serves as TypePtr::TOP
Align return.
src/hotspot/share/opto/compile.cpp line 1778:
> 1776: bool Compile::can_alias(const TypePtr* adr_type, int alias_idx) {
> 1777: if (alias_idx == AliasIdxTop) return false; // the empty category
> 1778: if (adr_type == nullptr) return false; // null serves as TypePtr::TOP
Align returns.
src/hotspot/share/opto/divnode.cpp line 467:
> 465: const Type *t = phase->type( in(2) );
> 466: if( t == TypeInt::ONE ) // Identity?
> 467: return nullptr; // Skip it
Align comment.
src/hotspot/share/opto/divnode.cpp line 482:
> 480: jint i = ti->get_con(); // Get divisor
> 481:
> 482: if (i == 0) return nullptr; // Dividing by zero constant does not idealize
Align comment? With line 480.
src/hotspot/share/opto/divnode.cpp line 573:
> 571: const Type *t = phase->type( in(2) );
> 572: if( t == TypeLong::ONE ) // Identity?
> 573: return nullptr; // Skip it
Align comment.
src/hotspot/share/opto/divnode.cpp line 581:
> 579: // Check for excluding div-zero case
> 580: if (in(0) && (tl->_hi < 0 || tl->_lo > 0)) {
> 581: set_req(0, nullptr); // Yank control input
Align comment.
src/hotspot/share/opto/divnode.cpp line 588:
> 586: jlong l = tl->get_con(); // Get divisor
> 587:
> 588: if (l == 0) return nullptr; // Dividing by zero constant does not idealize
Align comment.
src/hotspot/share/opto/divnode.cpp line 724:
> 722: const Type *t2 = phase->type( in(2) );
> 723: if( t2 == TypeF::ONE ) // Identity?
> 724: return nullptr; // Skip it
Align comment.
src/hotspot/share/opto/divnode.cpp line 816:
> 814: const Type *t2 = phase->type( in(2) );
> 815: if( t2 == TypeD::ONE ) // Identity?
> 816: return nullptr; // Skip it
Align comment.
src/hotspot/share/opto/gcm.cpp line 283:
> 281: static Block* find_deepest_input(Node* n, const PhaseCFG* cfg) {
> 282: // Find the last input dominated by all other inputs.
> 283: Block* deepb = nullptr; // Deepest block so far
Align comment.
src/hotspot/share/opto/gcm.cpp line 431:
> 429: static Block* raise_LCA_above_use(Block* LCA, Node* use, Node* def, const PhaseCFG* cfg) {
> 430: Block* buse = cfg->get_block_for_node(use);
> 431: if (buse == nullptr) return LCA; // Unused killing Projs have no use block
Align return.
src/hotspot/share/opto/graphKit.cpp line 178:
> 176: // Tell if _map is null, or control is top.
> 177: bool GraphKit::stopped() {
> 178: if (map() == nullptr) return true;
Align return.
src/hotspot/share/opto/library_call.cpp line 2417:
> 2415: (mismatched ||
> 2416: heap_base_oop == top() || // - heap_base_oop is null or
> 2417: (can_access_non_heap && field == nullptr)) // - heap_base_oop is potentially null
Align comment.
src/hotspot/share/opto/loopnode.cpp line 470:
> 468: if (!stride->is_Con()) { // Oops, swap these
> 469: if (!xphi->is_Con()) { // Is the other guy a constant?
> 470: return nullptr; // Nope, unknown stride, bail out
Align comment.
src/hotspot/share/opto/memnode.cpp line 271:
> 269: st->print("alias_idx==%d, adr_check==", alias_idx);
> 270: if( adr_check == nullptr ) {
> 271: st->print("null");
Where are these strings printed? Is this a user detectable change?
src/hotspot/share/opto/memnode.cpp line 3697:
> 3695: Node* base = AddPNode::Ideal_base_and_offset(st->in(MemNode::Address),
> 3696: phase, offset);
> 3697: if (base == nullptr) return -1; // something is dead,
Align returns.
src/hotspot/share/opto/memnode.cpp line 3720:
> 3718:
> 3719: Node* n = worklist.at(j);
> 3720: if (n == nullptr) continue; // (can this really happen?)
Align continue.
src/hotspot/share/opto/memnode.cpp line 3944:
> 3942: int i = captured_store_insertion_point(start, size_in_bytes, phase);
> 3943: if (i == 0) {
> 3944: return nullptr; // something is dead
Align comment.
src/hotspot/share/opto/memnode.cpp line 3995:
> 3993: int i = captured_store_insertion_point(start, size_in_bytes, phase);
> 3994: if (i == 0) return nullptr; // bail out
> 3995: Node* prev_mem = nullptr; // raw memory for the captured store
Align comments.
src/hotspot/share/opto/memnode.cpp line 4166:
> 4164: intcon[0] = 0; // undo store_constant()
> 4165: set_req(i-1, st); // undo set_req(i, zmem)
> 4166: nodes[j] = nullptr; // undo nodes[j] = st
Align comment.
src/hotspot/share/opto/multnode.cpp line 158:
> 156: void ProjNode::check_con() const {
> 157: Node* n = in(0);
> 158: if (n == nullptr) return; // should be assert, but NodeHash makes bogons
Align return.
src/hotspot/share/opto/node.cpp line 1050:
> 1048: i++;
> 1049: }
> 1050: _in[i] = n; // Stuff prec edge over null
Align comment.
src/hotspot/share/opto/node.hpp line 529:
> 527: }
> 528: _in[gap] = last; // Move last slot to empty one.
> 529: _in[i] = nullptr; // null out last slot.
Align comment.
src/hotspot/share/opto/output.cpp line 2158:
> 2156: #ifndef PRODUCT
> 2157: if (_cfg->C->trace_opto_output())
> 2158: tty->print("# ChooseNodeToBundle: null\n");
User observable change(?) Do we care?
src/hotspot/share/opto/phaseX.cpp line 2292:
> 2290: } else if( n->is_Region() ) { // Unreachable region
> 2291: // Note: nn == C->top()
> 2292: n->set_req(0, nullptr); // Cut selfreference
Align comment.
-------------
Marked as reviewed by jwilhelm (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12187
More information about the hotspot-compiler-dev
mailing list