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

[Christian Thalinger]

On Nov 16, 2010, at 6:35 PM, Vladimir Kozlov wrote:
> On 11/16/10 2:35 AM, Christian Thalinger wrote:
>> 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?
>
> I think you should move Constant class and table into Compile class.

Will do.

>
>>
>>>> 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.
>
> Can you explain more why you need code offset of MCBN? I thought you  
> need only base of constant table in register produced by this node.  
> You collect all constants information (filling constant table)  
> before emitting code so you should know all offsets in the table  
> before emit.


My fault.  I was thinking that we're using RDPC when the node is not  
pinned, but that's not the case.  Of course it works when not using  
RDPC.

-- Christian