Request for reviews (XXL): 6961690: load oops from constant table on SPARC

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Nov 12 12:19:42 PST 2010


Do you need to connect MachConstantBaseNode to root? For RA? And where you emit it (call emit()) since you don't add it to any block:

+MachConstantBaseNode* Compile::mach_constant_base_node() {
+  if (_mach_constant_base_node == NULL) {
+    _mach_constant_base_node = new (C) MachConstantBaseNode();
+    _mach_constant_base_node->set_req(0, C->root());

Why commented?:
!     //assert(!n->pinned() || n->is_SafePointScalarObject(), "only SafePointScalarObject pinned node is expected here");

You don't use is_MachConstantBase() so you don't need to add it to DEFINE_CLASS in node.hpp and call init_class_id().

May be one line?:

+// Machine node that holds a constant which is stored in the constant
+// table.

You don't need to check for is_Mach(), check directly n->is_MachConstant():

+        if (n->is_Mach()) {
+          MachNode *mach = n->as_Mach();
+
+          // If the MachNode is a MachConstantNode evaluate the
+          // constant value section.
+          if (mach->is_MachConstant()) {
+            MachConstantNode* machcon = mach->as_MachConstant();
+            machcon->eval_constant();
+          }
+        }

Vladimir

Christian Thalinger wrote:
> http://cr.openjdk.java.net/~twisti/6961690/webrev.01/
> 
> 6961690: load oops from constant table on SPARC
> Summary: oops should be loaded from the constant table of an nmethod 
> instead of materializing them with a long code sequence.
> Reviewed-by:
> 
> oops should be loaded from the constant table of an nmethod instead of
> materializing them with a long code sequence.
> 
> This fix introduces two new ideal nodes MachConstantBaseNode and
> MachConstantNode.  MachConstantBaseNode represents the base address of
> the constant table, MachConstantNode a constant that should be read
> from the constant table.
> 
> ADLC was changed to support three new keywords: constantaddress,
> constanttablebase, and constantoffset.  constantaddress represents the
> absolute address of a constant and is used on x86.  constanttablebase
> represents the register that holds the constant table base address and
> constantoffset is the relative offset to constanttablebase.  The
> latter two are used on SPARC.
> 
> For SPARC a new command line option was added:
> UseRDPCForConstantTableBase.  If that option is true the RDPC
> instruction is used to get the constant table base address instead of
> materializing the address.  This option is currently off by default
> and may be switched on for future SPARC implementations.
> 
> The patch also includes a change from Tom that fixes a bug that was
> uncovered by this work:
> 
> http://cr.openjdk.java.net/~twisti/6961690/webrev.01/src/share/vm/opto/postaloc.cpp.udiff.html 
> 
> 
> src/cpu/sparc/vm/assembler_sparc.cpp
> src/cpu/sparc/vm/assembler_sparc.hpp
> src/cpu/sparc/vm/assembler_sparc.inline.hpp
> src/cpu/sparc/vm/sparc.ad
> src/cpu/x86/vm/assembler_x86.cpp
> src/cpu/x86/vm/assembler_x86.hpp
> src/cpu/x86/vm/x86_32.ad
> src/cpu/x86/vm/x86_64.ad
> src/os/linux/vm/vmError_linux.cpp
> src/share/vm/adlc/adlparse.cpp
> src/share/vm/adlc/adlparse.hpp
> src/share/vm/adlc/archDesc.hpp
> src/share/vm/adlc/formssel.cpp
> src/share/vm/adlc/formssel.hpp
> src/share/vm/adlc/output_c.cpp
> src/share/vm/adlc/output_h.cpp
> src/share/vm/asm/assembler.hpp
> src/share/vm/asm/assembler.inline.hpp
> src/share/vm/compiler/disassembler.cpp
> src/share/vm/opto/c2_globals.hpp
> src/share/vm/opto/compile.cpp
> src/share/vm/opto/compile.hpp
> src/share/vm/opto/gcm.cpp
> src/share/vm/opto/machnode.cpp
> src/share/vm/opto/machnode.hpp
> src/share/vm/opto/node.hpp
> src/share/vm/opto/output.cpp
> src/share/vm/opto/postaloc.cpp
> src/share/vm/utilities/debug.cpp
> 


More information about the hotspot-compiler-dev mailing list