RFR(XS): 8028109: compiler/codecache/CheckReservedInitialCodeCacheSizeArgOrder.java crashes in RT_Baseline

[Albert Noll]

Igor, Vladimir, thanks for the reviews.

Here is a version that uses 'mov' for 32-bit and 64-bit to load 
'byte_map_base'.

http://cr.openjdk.java.net/~anoll/8028109/webrev.02/

Best,
Albert

On 11/19/2013 09:31 PM, Igor Veresov wrote:
> Indeed, I agree, a move would be more straightforward.
> Later on we’ll need a special type of relocation added for the cardtable pointer anyway, because it has very unique rules to compute it depending on both location of the heap and the cardtable array. It is certainly not an ExternalAddress.
>
> igor
>
> On Nov 19, 2013, at 12:08 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> On 11/19/13 11:57 AM, Igor Veresov wrote:
>>> Except we will need some relocation info for it if we plan to do snapshotting.
>> True, but after Tom's fix it does not have a relocation in all cases already:
>>
>> http://cr.openjdk.java.net/~never/6777083/src/cpu/x86/vm/assembler_x86.hpp.udiff.html
>>
>> And I think, there are places where we don't use ExternalAddress so we may need to solve serialization differently later.
>>
>> Thanks,
>> Vladimir
>>
>>> igor
>>>
>>> On Nov 19, 2013, at 11:36 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>
>>>> Last version is not good because it may hide bug when wrong address matches byte_map_base value. Original Tom's fix also hides problems.
>>>>
>>>> MacroAssembler::g1_write_barrier_post() and store_check_part_2() are used only in interpreter. It does not make sense to optimize it. Note, x86 has limited throughput for address calculations. And one instruction is only generated if this address is 32-bit which may be not true then you have to move it into register too.
>>>>
>>>> Anyway, we can go case by case and avoid ExternalAddress in 64-bit VM because code allows us to do that:
>>>>
>>>> In both cases in c1_Runtime1_x86.cpp and in MacroAssembler::g1_write_barrier_post() there is separate lea() instruction for LP64 which could be replaced with mov to load constant.
>>>>
>>>> MacroAssembler::store_check_part_2() is different case which we may prefer to use in other cases. Note, as_Address(ArrayAddress adr) generates lea() instruction again. Which means code for 32-bit VM in c1_Runtime1_x86.cpp generates 2 lea instructions and not one:
>>>>
>>>> __ leal(card_addr, __ as_Address(ArrayAddress(cardtable, index)));
>>>>
>>>> What I am saying is lets use mov instruction to load byte_map_base instead of using ExternalAddress because we don't need relocation for it and because in reality (with all past fixes) we will generate the same number of instructions.
>>>>
>>>> Regards,
>>>> Vladimir
>>>>
>>>> On 11/19/13 7:59 AM, Albert Noll wrote:
>>>>> Hi Roland,
>>>>>
>>>>> thanks for pointing this out. Here is a patch that executes the assert
>>>>> only if
>>>>> the address we are checking is not byte_map_base. This might not be the
>>>>> most
>>>>> elegant solution, but at least it seems to work.
>>>>>
>>>>> Here is the new webrev:
>>>>> http://cr.openjdk.java.net/~anoll/8028109/webrev.01/
>>>>>
>>>>> Best,
>>>>> Albert
>>>>>
>>>>>
>>>>> On 11/19/2013 10:00 AM, Roland Westrelin wrote:
>>>>>>> Solution: Change 'ExternalAddress' to 'AddressLiteral' that generates
>>>>>>> no relocation information for
>>>>>>>                byte_map_base. Other platforms also AddressLiteral
>>>>>>> without relocation information.
>>>>>> As mentioned in the thread here:
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2011-April/005151.html
>>>>>>
>>>>>> in this message:
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2011-April/005161.html
>>>>>>
>>>>>>
>>>>>> not using ExternalAddress disables rip relative addressing.
>>>>>> This said, I’m not sure what a clean fix for this would be.
>>>>>>
>>>>>> Roland.