JDK-8301497: Replace NULL with nullptr in cpu/s390 #12325

jdksjolen · 2023-01-31T11:40:09Z

Hi, this PR changes all occurrences of NULL to nullptr for the subdirectory cpu/s390. 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!

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8301497: Replace NULL with nullptr in cpu/s390

Reviewers

Amit Kumar (@offamitkumar - Author)
Coleen Phillimore (@coleenp - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/12325/head:pull/12325
$ git checkout pull/12325

Update a local copy of the PR:
$ git checkout pull/12325
$ git pull https://git.openjdk.org/jdk.git pull/12325/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12325

View PR using the GUI difftool:
$ git pr show -t 12325

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/12325.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-01-31T11:41:04Z

👋 Welcome back jsjolen! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-01-31T11:44:22Z

@jdksjolen The following label will be automatically applied to this pull request:

hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

jdksjolen

Reviewed and found a decent chunk of things to fix.

jdksjolen · 2023-02-13T09:31:34Z

src/hotspot/cpu/s390/abstractInterpreter_s390.cpp

@@ -121,9 +121,9 @@ int AbstractInterpreter::size_activation(int max_stack,
 //
 // Parameters:
 //
-// interpreter_frame != NULL:
+// interpreter_frame != null:


jdksjolen · 2023-02-13T10:03:02Z

src/hotspot/cpu/s390/frame_s390.cpp

@@ -111,7 +111,7 @@ bool frame::safe_for_sender(JavaThread *thread) {
    }

    // At this point, there still is a chance that fp_safe is false.
-    // In particular, (fp == NULL) might be true. So let's check and
+    // In particular, (fp == null) might be true. So let's check and


jdksjolen · 2023-02-13T10:03:34Z

src/hotspot/cpu/s390/gc/g1/g1BarrierSetAssembler_s390.cpp

@@ -404,7 +404,7 @@ void G1BarrierSetAssembler::resolve_jobject(MacroAssembler* masm, Register value
  __ z_braz(Lnot_weak);
  __ verify_oop(value, FILE_AND_LINE);
  DecoratorSet decorators = IN_NATIVE | ON_PHANTOM_OOP_REF;
-  g1_write_barrier_pre(masm, decorators, (const Address*)NULL, value, noreg, tmp1, tmp2, true);
+  g1_write_barrier_pre(masm, decorators, (const Address*)nullptr, value, noreg, tmp1, tmp2, true);


unnecessary cast?

jdksjolen · 2023-02-13T10:04:06Z

src/hotspot/cpu/s390/interp_masm_s390.cpp

@@ -275,7 +275,7 @@ void InterpreterMacroAssembler::check_and_handle_earlyret(Register scratch_reg)
    Register jvmti_thread_state = Z_ARG2;
    Register tmp                = Z_ARG3;
    load_and_test_long(jvmti_thread_state, Address(Z_thread, JavaThread::jvmti_thread_state_offset()));
-    z_bre(L); // if (thread->jvmti_thread_state() == NULL) exit;
+    z_bre(L); // if (thread->jvmti_thread_state() == null) exit;


jdksjolen · 2023-02-13T10:04:16Z

src/hotspot/cpu/s390/interp_masm_s390.cpp

@@ -981,7 +981,7 @@ void InterpreterMacroAssembler::lock_object(Register monitor, Register object) {
  //   // We stored the monitor address into the object's mark word.
  // } else if (THREAD->is_lock_owned((address)displaced_header))
  //   // Simple recursive case.
-  //   monitor->lock()->set_displaced_header(NULL);
+  //   monitor->lock()->set_displaced_header(null);


jdksjolen · 2023-02-13T10:14:42Z

src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp

-  // z_ijava_state->oop_temp = NULL;
+  // z_ijava_state->oop_temp = null;
  __ store_const(Address(fp, oop_tmp_offset), 0);

  // Initialize z_ijava_state->mdx.
  Register Rmdp = Z_bcp;
-  // native_call: assert that mdo == NULL
+  // native_call: assert that mdo == null


jdksjolen · 2023-02-13T10:15:14Z

src/hotspot/cpu/s390/templateTable_s390.cpp

+  // tos = 0: obj == null or  obj is not an instanceof the specified klass
+  // tos = 1: obj != null and obj is     an instanceof the specified klass


jdksjolen · 2023-02-13T10:15:24Z

src/hotspot/cpu/s390/templateTable_s390.cpp

@@ -4140,7 +4140,7 @@ void TemplateTable::monitorenter() {
    __ bind(exit);
  }

-  // Rfree_slot != NULL -> found one
+  // Rfree_slot != null -> found one


jdksjolen · 2023-02-13T10:16:01Z

src/hotspot/cpu/s390/vtableStubs_s390.cpp

@@ -82,7 +82,7 @@ VtableStub* VtableStubs::create_vtable_stub(int vtable_index) {
  assert(VtableStub::receiver_location() == Z_R2->as_VMReg(), "receiver expected in Z_ARG1");

  const Register rcvr_klass   = Z_R1_scratch;
-  address        npe_addr     = __ pc(); // npe == NULL ptr exception
+  address        npe_addr     = __ pc(); // npe == null ptr exception


This reads a bit strange.

Read this as "npe means null pointer exception" or however you would like to spell it.

So it's saying "NPE is short for null pointer exception"?

jdksjolen · 2023-02-13T10:16:12Z

src/hotspot/cpu/s390/vtableStubs_s390.cpp

@@ -195,7 +195,7 @@ VtableStub* VtableStubs::create_itable_stub(int itable_index) {

  // Get receiver klass.
  // Must do an explicit check if offset too large or implicit checks are disabled.
-  address npe_addr = __ pc(); // npe == NULL ptr exception
+  address npe_addr = __ pc(); // npe == null ptr exception


This also reads a bit strange

Read this as "npe means null pointer exception" or however you would like to spell it.

openjdk · 2023-02-17T18:56:57Z

@jdksjolen this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8301497
git fetch https://git.openjdk.org/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

jdksjolen · 2023-03-21T09:52:25Z

Don't close yet.

mlbridge · 2023-04-11T10:11:44Z

Webrevs

00: Full (35977991)

offamitkumar

I have tested release & fast debug build. Tier1 test on fastdebug as well. PR seems clean.

Thank you so much for the changes.

coleenp

Looks good!

openjdk · 2023-04-17T18:48:16Z

@jdksjolen This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8301497: Replace NULL with nullptr in cpu/s390

Reviewed-by: amitkumar, coleenp

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 68 new commits pushed to the master branch:

6831f9d: 8278583: Open source SwingMark - Swing performance benchmark
497f9e7: 8305755: [JVMCI] missing barriers in CompilerToVM.readFieldValue for Reference.referent
4ed933c: 8296248: Update CLDR to Version 43.0
7360960: 8305625: Stress test crashes with SEGV in Deoptimization::deoptimize_frame_internal(JavaThread*, long*, Deoptimization::DeoptReason)
cc60f2f: 8305060: G1: Refactor G1ScanHRForRegionClosure::scan_heap_roots
fb58d77: 8305192: serial GC fails "assert(Universe::on_page_boundary(bottom) && Universe::on_page_boundary(end)) failed: invalid space boundaries"
7551529: 8305995: Footprint regression from JDK-8224957
02347d0: 8305351: C2 setScopedValueCache intrinsic doesn't use access API
2240c7e: 8305543: Ensure GC barriers for arraycopy on AArch64 use caller saved neon temp registers
1958f0e: 8305233: G1: Refactor G1ClearCardTableTask
... and 58 more: https://git.openjdk.org/jdk/compare/cd7d53c88c27eedbe16020b88c2219708d170a1e...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

jdksjolen · 2023-04-18T08:58:32Z

Thanks!

/integrate

openjdk · 2023-04-18T08:59:39Z

Going to push as commit 54f7b6c.
Since your change was applied there have been 75 commits pushed to the master branch:

8ecb5df: 8305781: compiler/c2/irTests/TestVectorizationMultiInvar.java failed with "IRViolationException: There were one or multiple IR rule failures."
49726ee: 8305690: [X86] Do not emit two REX prefixes in Assembler::prefix
445ebef: 8305668: PPC: Non-Top Interpreted frames should be independent of ABI_ELFv2
e3ece36: 8303422: Use common functions to exit the VM for -Xshare:dump and CDS errors
bb1a7bb: 8305762: FileInputStream and FileOutputStream implSpec should be corrected or removed
8858d54: 8305811: (bf) Improve performance of CharBuffer::append(CharSequence[,int,int])
525a91e: 8305673: Convert DocCommentParser to use enhanced switch
6831f9d: 8278583: Open source SwingMark - Swing performance benchmark
497f9e7: 8305755: [JVMCI] missing barriers in CompilerToVM.readFieldValue for Reference.referent
4ed933c: 8296248: Update CLDR to Version 43.0
... and 65 more: https://git.openjdk.org/jdk/compare/cd7d53c88c27eedbe16020b88c2219708d170a1e...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-04-18T08:59:53Z

@jdksjolen Pushed as commit 54f7b6c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Replace NULL with nullptr in cpu/s390

6b117ea

openjdk bot added the hotspot hotspot-dev@openjdk.org label Jan 31, 2023

jdksjolen commented Feb 13, 2023

View reviewed changes

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Feb 17, 2023

jdksjolen added 2 commits April 11, 2023 11:59

Fixes

24de7ed

Merge remote-tracking branch 'origin/master' into JDK-8301497

3597799

jdksjolen marked this pull request as ready for review April 11, 2023 10:05

openjdk bot added rfr Pull request is ready for review and removed merge-conflict Pull request has merge conflict with target branch labels Apr 11, 2023

offamitkumar approved these changes Apr 12, 2023

View reviewed changes

coleenp approved these changes Apr 17, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 17, 2023

openjdk bot added the integrated Pull request has been integrated label Apr 18, 2023

openjdk bot closed this Apr 18, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 18, 2023

offamitkumar mentioned this pull request Apr 19, 2023

8306459: s390x: Replace NULL to nullptr #13538

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8301497: Replace NULL with nullptr in cpu/s390 #12325

JDK-8301497: Replace NULL with nullptr in cpu/s390 #12325

jdksjolen commented Jan 31, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 31, 2023

openjdk bot commented Jan 31, 2023

jdksjolen left a comment

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

jdksjolen Feb 13, 2023

RealLucy Feb 14, 2023

jdksjolen Apr 11, 2023

jdksjolen Feb 13, 2023

RealLucy Feb 14, 2023

openjdk bot commented Feb 17, 2023

jdksjolen commented Mar 21, 2023

mlbridge bot commented Apr 11, 2023

offamitkumar left a comment

coleenp left a comment

openjdk bot commented Apr 17, 2023

jdksjolen commented Apr 18, 2023

openjdk bot commented Apr 18, 2023

openjdk bot commented Apr 18, 2023

		// tos = 0: obj == null or obj is not an instanceof the specified klass
		// tos = 1: obj != null and obj is an instanceof the specified klass

JDK-8301497: Replace NULL with nullptr in cpu/s390 #12325

JDK-8301497: Replace NULL with nullptr in cpu/s390 #12325

Conversation

jdksjolen commented Jan 31, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jan 31, 2023

openjdk bot commented Jan 31, 2023

jdksjolen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openjdk bot commented Feb 17, 2023

jdksjolen commented Mar 21, 2023

mlbridge bot commented Apr 11, 2023

Webrevs

offamitkumar left a comment

Choose a reason for hiding this comment

coleenp left a comment

Choose a reason for hiding this comment

openjdk bot commented Apr 17, 2023

jdksjolen commented Apr 18, 2023

openjdk bot commented Apr 18, 2023

openjdk bot commented Apr 18, 2023

jdksjolen commented Jan 31, 2023 •

edited by openjdk bot

Loading