RFR(M): 8138952: C1: Distinguish between PPC32 and PPC64
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Mon Nov 23 11:57:37 UTC 2015
I'll push it today.
Best regards,
Vladimir Ivanov
On 11/23/15 1:18 PM, Doerr, Martin wrote:
> Thanks for the 2nd review.
>
> The new webrev with updated copyright and reviewer attribution is here:
>
> http://cr.openjdk.java.net/~mdoerr/8138952_c1_PPC64_defs/webrev.03/
>
> Can anybody sponsor, please?
>
> Thanks and best regards,
>
> Martin
>
> *From:*Lindenmaier, Goetz
> *Sent:* Montag, 23. November 2015 10:54
> *To:* Doerr, Martin <martin.doerr at sap.com>; 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 Martin,
>
> thanks for doing this change.
>
> You need to adapt the copyright in the x86/sparc platform files you
>
> touch after the last update.
>
> Besides that it looks good.
>
> Best regards,
>
> Goetz.
>
> *From:*hotspot-compiler-dev
> [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] *On Behalf Of
> *Doerr, Martin
> *Sent:* Freitag, 20. November 2015 18:16
> *To:* Christian Thalinger
> *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
>
> Hi Christian,
>
> thanks for looking at it. I found a way to move the PPC64 special stuff
> out of the shared code.
>
> I’m passing the stub pointer to the null_check which I added to the
> platform c1_MacroAssembler files.
>
> PPC64 can do the required stuff inside of it and the other platforms
> just delegate to the MacroAssembler::null_check.
>
> The new webrev is here:
>
> http://cr.openjdk.java.net/~mdoerr/8138952_c1_PPC64_defs/webrev.02/
>
> Best regards,
>
> Martin
>
> *From:*Christian Thalinger [mailto:christian.thalinger at oracle.com]
> *Sent:* Donnerstag, 19. November 2015 19:20
> *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
>
> On Nov 17, 2015, at 3:18 AM, Doerr, Martin <martin.doerr at sap.com
> <mailto:martin.doerr at sap.com>> wrote:
>
> 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?
>
> Yes but shouldn’t it depend on ImplicitNullChecks?
>
> Best regards,
>
> Martin
>
> *From:*Doerr, Martin
> *Sent:*Freitag, 13. November 2015 17:28
> *To:*'Christian Thalinger' <christian.thalinger at oracle.com
> <mailto:christian.thalinger at oracle.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
>
> 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
>
More information about the hotspot-compiler-dev
mailing list