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