RFR(M): 8138952: C1: Distinguish between PPC32 and PPC64

[Doerr, Martin]

Hi,

we have thought again about how we can get rid of the #ifdef PPC64 in the null check code (2.).
The idea is to implement the lir_null_check as follows:

      if (GenerateCompilerNullChecks) {
        if (!os::zero_page_read_protected() && !TrapBasedNullChecks) {
          assert(op->in_opr()->is_single_cpu(), "expected");
          explicit_null_check(op->in_opr()->as_register(), op->info());
        } else {
          add_debug_info_for_null_check_here(op->info());

          if (op->in_opr()->is_single_cpu()) {
            _masm->null_check(op->in_opr()->as_register());
          } else {
            Unimplemented();
          }
        }
      }

It handles the case “AIX without SIGTRAP” separately.
Disadvantage is that we will have to introduce explicit_null_check(Register, CodeEmitInfo*) on all other platforms with ShouldNotReachHere().

Is this better than the #ifdef PPC64?

Best regards,
  Martin


From: Doerr, Martin
Sent: Freitag, 13. November 2015 17:28
To: 'Christian Thalinger' <christian.thalinger at oracle.com>
Cc: hotspot-compiler-dev at openjdk.java.net
Subject: RE: RFR(M): 8138952: C1: Distinguish between PPC32 and PPC64

Hi Christian,

thank you for reviewing.

Here my explanations for your 3 points (which I have marked in your mail):

1.
I had made the change from #ifdef PPC to #if defined(PPC) intentionally, because we use the same code for other (not contributed) platforms as well and it’s easier to add platforms this way. Can you live with this as well? If not, I’ll remove it.

2.
I think the #ifdef PPC64 for the TrapBasedNullChecks case is not so nice, but it does the right thing.
PPC64 is the only platform with doesn’t fully support signal-based null checks. The normal null_check should work fine for all other platforms. PPC64 (especially AIX, see 3.) needs a compare instruction if SIGTRAP is not used. In this case, the null check info needs to get passed.
The version of explicit_null_check which takes a register and the CodeEmitInfo is currently only implemented in our PPC64 code.
We could move it to shared code, but other platforms don’t need it at the moment.

3.
TrapBasedNullChecks is a feature which is currently only implemented on PPC64. It enables the processor’s conditional trap instructions for null checks. It’s especially useful on AIX because the address 0 is readable (but not writeable) so we can’t use SEGV-based implicit null checks for loads.

Please let me know which changes to the webrev you would still like to see.

Best regards,
  Martin


From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
Sent: Freitag, 13. November 2015 05:48
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(M): 8138952: C1: Distinguish between PPC32 and PPC64

I remember having looked at this before.

src/share/vm/c1/c1_LIR.hpp:

-#ifdef PPC
+#if defined(PPC)
-#endif // PPC
+#endif
1. Please undo these changes.

src/share/vm/c1/c1_LIRAssembler.cpp:

     case lir_null_check:
       if (GenerateCompilerNullChecks) {
+#ifdef PPC64
+        if (!TrapBasedNullChecks) {
+          assert(op->in_opr()->is_single_cpu(), "expected");
+          explicit_null_check(op->in_opr()->as_register(), op->info());
+          break;
+        }
+#endif

2. Do we really need the ifdef?  Shouldn’t the code work on other architectures too?  I see that TrapBasedNullChecks is false on all architectures except PPC:

src/closed/cpu/arm/vm/globals_arm.hpp:19:define_pd_global(bool,  TrapBasedNullChecks,      false); // Not needed
src/cpu/aarch64/vm/globals_aarch64.hpp:40:define_pd_global(bool, TrapBasedNullChecks,  false);
src/cpu/ppc/vm/globals_ppc.hpp:41:define_pd_global(bool, TrapBasedNullChecks,   true);
src/cpu/sparc/vm/globals_sparc.hpp:45:define_pd_global(bool, TrapBasedNullChecks,         false); // Not needed on sparc.
src/cpu/x86/vm/globals_x86.hpp:39:define_pd_global(bool, TrapBasedNullChecks,      false); // Not needed on x86.
src/cpu/zero/vm/globals_zero.hpp:40:define_pd_global(bool,  TrapBasedNullChecks,  false);

3. Shouldn’t all be true?

On Nov 12, 2015, at 12:07 AM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:

Hi,

we already have a local openjdk with C1 on PPC64. However, we can’t contribute it without these shared code changes.
This one should not change anything for PPC32 but change the defines as needed for PPC64.

Can someone review and sponsor, please?

Best regards,
  Martin

From: Doerr, Martin
Sent: Dienstag, 3. November 2015 16:13
To: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: RFR(M): 8138952: C1: Distinguish between PPC32 and PPC64

Hi,

The shared C1 code currently uses many preprocessor directives which have been designed to work for the PPC32 port. In order to support our C1 port on PPC64 as well, some #ifdef etc. need modification.
Webrev is here:

http://cr.openjdk.java.net/~mdoerr/8138952_c1_PPC64_defs/webrev.01/

PPC is defined if either PPC32 and PPC64 is defined so we use it for code which is used by both ports.
I made the following changes to webrev.00:
I changed c1_LIR.hpp to use PPC for the method defined twice.
I removed the modification of the defines in c1_Compilation.hpp (it’s not needed).

Please review this change.  I need a sponsor, please.

Best regards,
  Martin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151117/eba0e1a2/attachment-0001.html>