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

[Albert Noll]

Thank you for the review, Vladimir. 

I also want to thank Roland who give me early feedback for this version.

Best,
Albert

Von meinem iPhone gesendet

> Am 27.11.2013 um 19:23 schrieb Vladimir Kozlov <vladimir.kozlov at oracle.com>:
> 
> Thank you, Albert, for additional comments.
> 
> Fix looks good.
> 
> Thanks,
> Vladimir
> 
>> On 11/27/13 8:26 AM, Albert Noll wrote:
>> 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.
>>