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

Christian Thalinger christian.thalinger at oracle.com
Tue Nov 16 02:35:28 PST 2010


On Nov 15, 2010, at 11:46 PM, Vladimir Kozlov wrote:
> Christian Thalinger wrote:
>> On Nov 12, 2010, at 9:19 PM, Vladimir Kozlov wrote:
>>> 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());
>> The MachConstantBaseNode is added as an input to all  
>> MachConstantNodes in their Expand method (generated by ADLC) and is  
>> emitted when all other nodes are emitted in Compile::Fill_buffer.
>
> I see it now. What confused me is you used this node for two different
> purposes. One is as Mach node to put into register the base of  
> constant
> section and second is to collect constants information during code
> generation (in output.cpp). I think we should separate them.

Well, yes, we can separate it.  You are right that the name of  
MachConstantBaseNode does not suggest that it also contains the whole  
constant table.  Would you either want me to (a) link from the  
MachConstantBaseNode to the new constant table object or (b) have a  
singleton (like _mach_constant_base_node) and access it via Compile?   
Where should I put the constant table class?

>> A change we could do (but I haven't tested this yet) is to not pin  
>> MachConstantBaseNodes when UseRDPCForConstanTableBase is false  
>> since we materialize the constant table base address anyway so it  
>> doesn't matter where in the code that happens.  Moving the  
>> materialization closer to the use would also decrease register  
>> pressure.
>
> I agree so, please, fix it.


I'm not sure that works.  One problem I hit is that a  
MachConstantBaseNode is in a basic block that is scheduled later in  
the block layout than a block that has a MachConstantNode and uses  
MachConstantBaseNode.  So when emitting the MachConstantNode we don't  
know the code offset of the MachConstantBaseNode yet and we can't emit  
the correct load offset.  I don't see a way to fix this without  
changing the block layout which definitely is not a good idea.

-- Christian


More information about the hotspot-compiler-dev mailing list