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

[Vladimir Kozlov]

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.

>
>>> 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.

Thanks,
Vladimir

>
> -- Christian