RFR 8150388: Remove SPARC 32-bit support
Kim Barrett
kim.barrett at oracle.com
Sat Apr 15 08:27:56 UTC 2017
> On Apr 7, 2017, at 10:47 AM, George Triantafillou <george.triantafillou at oracle.com> wrote:
>
> (Adding compiler).
> Please review this fix to remove SPARC 32-bit support. Support for solaris-sparc has been dropped from the list of supported platforms.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8150388
> webrev: http://cr.openjdk.java.net/~gtriantafill/8150388-webrev/webrev/ <http://cr.openjdk.java.net/%7Egtriantafill/8150388-webrev/webrev/>
>
> Built and tested on solaris-sparcv9-debug,solaris-x64-debug with the nsk.jvmti, nsk.jdwp, and nsk.jdi testlists.
>
> Thanks.
>
> -George
That's a lot of files and changes! I hope you'll be providing
incremental webrevs for updates.
------------------------------------------------------------------------------
src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
1393 // the poll may need a register so just pick one that isn't the return register
This comment appears to refer to the 32bit code that followed it and
was removed. I think the comment should be removed too.
------------------------------------------------------------------------------
src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
1490 Register yhi = opr2->as_register_hi();
After the removal of the 32bit code, I don't see any remaining
references to yhi. Not sure what that implies, other than there's
likely further work to do here.
------------------------------------------------------------------------------
src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
1559 } else
1560 __ brx(acond, false, Assembler::pt, skip); // checks icc on 32bit and xcc on 64bit
Please add braces to that else-clause. They were missing before
because the if/else was 64bit-only, and the close brace would have
also needed 64bit conditionalization.
------------------------------------------------------------------------------
src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
2574 Register cmp_value_hi = op->cmp_value()->as_register_hi();
...
2576 Register new_value_hi = op->new_value()->as_register_hi();
After the removal of the 32bit code, I don't see any remaining
references to the xxx_hi variables. Not sure what that implies, other
than there's likely further work to do here.
------------------------------------------------------------------------------
src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
3047 void LIR_Assembler::volatile_move_op(LIR_Opr src, LIR_Opr dest, BasicType type, CodeEmitInfo* info) {
3048 ShouldNotReachHere();
3049
3050 NEEDS_CLEANUP;
It appears that in 64bit mode this function has no implementation, and
the following 50+ lines of code are presently 32bit-only. Likely
further work to do here.
------------------------------------------------------------------------------
rc/cpu/sparc/vm/copy_sparc.hpp
116 static void pd_conjoint_jlongs_atomic(jlong* from, jlong* to, size_t count) {
--- TBD
Removed call to _Copy_conjoint_jlongs_atomic. Check if it's still
needed elsewhere or definition is removed.
------------------------------------------------------------------------------
src/cpu/sparc/vm/globalDefinitions_sparc.hpp
43 #elif defined(COMPILER1)
44 // pure C1, 32-bit, small machine
45 #define DEFAULT_CACHE_LINE_SIZE 16
Comment suggests this is a configuration we don't use with 64bit.
------------------------------------------------------------------------------
src/cpu/sparc/vm/globals_sparc.hpp
59 // Stack slots are 2X larger in LP64 than in the 32 bit VM.
Comment seems dead now.
------------------------------------------------------------------------------
src/cpu/sparc/vm/macroAssembler_sparc.inline.hpp
There are a lot of operations here that are for dispatching on 32/64.
Some of them may be worth keeping for documentation purposes, but some
might be better off being removed. That kind of cleanup should
probably wait for a later RFE though.
------------------------------------------------------------------------------
src/cpu/sparc/vm/macroAssembler_sparc.inline.hpp
323 inline intptr_t MacroAssembler::load_pc_address( Register reg, int bytes_to_skip ) {
324 intptr_t thepc = (intptr_t)pc() + 2*BytesPerInstWord + bytes_to_skip;
325 Unimplemented();
Presumably never called in 64bit mode.
------------------------------------------------------------------------------
src/cpu/sparc/vm/sparc.ad
314 // 64-bit build means 64-bit pointers means hi/lo pairs
"64-bit build means" seems superfluous now.
------------------------------------------------------------------------------
src/cpu/sparc/vm/sparc.ad
355 // 32-bit, longs in 1 register: cannot use I's and L's. Restrict to O's and G's.
Comment seems superfluous now.
------------------------------------------------------------------------------
src/cpu/sparc/vm/sparc.ad
1856 // Note that we if-def off of _LP64.
1857 // The relevant question is how the int is callee-saved. In _LP64
1858 // the whole long is written but de-opt'ing will have to extract
1859 // the relevant 32 bits, in not-_LP64 only the low 32 bits is written.
The commentary here needs some updating; there is no not-_LP64
anymore.
------------------------------------------------------------------------------
src/cpu/sparc/vm/sparc.ad
1947 // The intptr_t operand types, defined by textual substitution.
1948 // (Cf. opto/type.hpp. This lets us avoid many, many other ifdefs.)
These comments suggest there are further cleanup opportunities,
probably for a later RFE.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
Removed block starting with:
831 #if defined(ASSERT) && defined(_LP64)
That looks like an incorrect removal.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
1232 if (!aligned)
1233 {
Please move the { to line 1232.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
1334 } else
1335 {
Please move the { to line 1334.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
1445 if (!aligned)
1446 {
Please move the { to line 1445.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
1562 if (!aligned) {
1563 // align to 8 bytes, we know we are 4 byte aligned to start
1564 __ andcc(to, 7, G0);
1565 __ br(Assembler::zero, false, Assembler::pt, L_fill_32_bytes);
1566 __ delayed()->nop();
1567 __ stw(value, to, 0);
1568 __ inc(to, 4);
1569 __ dec(count, 1 << shift);
1570 __ BIND(L_fill_32_bytes);
1571 }
Please fix the indentation.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
1777 } else
1778 {
Please move the { to line 1777.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
1887 if (!aligned)
1888 {
Please move the { to line 1887.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
3085 } else
3086 {
Please move the { to line 3085.
------------------------------------------------------------------------------
src/cpu/sparc/vm/stubGenerator_sparc.cpp
5103 // Write the high part of the address
5104 // [RGV] Check if there is a dependency on the size of this prolog
Pre-existing: These comments seem mis-indented. I wonder who RGV is,
and how old this comment is.
------------------------------------------------------------------------------
src/cpu/sparc/vm/templateInterpreterGenerator_sparc.cpp
60 // The sethi() instruction generates lots more instructions when shell
61 // stack limit is unlimited, so that's why this is much bigger.
"why this is much bigger" ? than what? Oh, the 32bit value. Some
revision of comment is called for here.
------------------------------------------------------------------------------
src/cpu/sparc/vm/vm_version_sparc.cpp
73 // 32-bit oops don't make sense for the 64-bit VM on sparc
74 // since the 32-bit VM has the same registers and smaller objects.
This comment probably needs updating.
------------------------------------------------------------------------------
src/os_cpu/solaris_sparc/vm/atomic_solaris_sparc.hpp
166 #if defined(COMPILER2) || defined(_LP64)
and
228 #endif // _LP64 || COMPILER2
I think the _LP64 parts of these can be removed.
------------------------------------------------------------------------------
src/os_cpu/solaris_sparc/vm/atomic_solaris_sparc.hpp
57 #ifdef _GNU_SOURCE
For a future clenaup, Solaris Studio now supports the gcc-style inline
assembler being protected by this #ifdef. We could probably eliminate
the alternative definitions, though if we're not already using the
inline versions they will need to be carefully examined first.
------------------------------------------------------------------------------
src/os_cpu/solaris_sparc/vm/prefetch_solaris_sparc.inline.hpp
30 #if defined(COMPILER2) || defined(_LP64)
and
58 #endif // defined(COMPILER2) || defined(_LP64)
I think the _LP64 parts of these can be removed.
------------------------------------------------------------------------------
More information about the hotspot-compiler-dev
mailing list