Request for review (S): 7032388: guarantee(VM_Version::supports_cmov()) failed: illegal instruction on i586 after 6919934

Paul Hohensee paul.hohensee at oracle.com
Wed Mar 30 14:22:55 PDT 2011


Yes.

Paul

On 3/30/11 3:34 PM, Igor Veresov wrote:
> Still looks good.
>
> igor
>
> On 3/30/11 11:30 AM, Christian Thalinger wrote:
>> I made two small changes that Paul suggested since #ifdefs are bad 
>> and cmovptr makes more sense in the context.
>>
>> -- Christian
>>
>> On Mar 30, 2011, at 5:40 PM, Paul Hohensee wrote:
>>> Couple of small comments.
>>>
>>> In assembler_x86.cpp, I'd removed the #ifdef _LP64 and have 
>>> supports_cmov
>>> be the condition that determines whether or not to emit the cmov.  
>>> Every 64-bit
>>> chip supports cmov, so supports_cmov will return true on every 
>>> 64-bit platform.
>>>
>>> In templateTable_x86_32.cpp, you use cmov, which you've made an 
>>> alias for
>>> cmovptr in assembler_x86.hpp.  I'd get use cmovptr directly here 
>>> instead,
>>> esp. since cmpptr is used just before it.
>>>
>>> Paul
>>>
>>> On 3/30/11 8:46 AM, Christian Thalinger wrote:
>>>> http://cr.openjdk.java.net/~twisti/7032388/
>>>>
>>>> 7032388: guarantee(VM_Version::supports_cmov()) failed: illegal 
>>>> instruction on i586 after 6919934
>>>> Summary: 6919934 added some unguarded cmov instructions which hit a 
>>>> guarantee on older hardware.
>>>> Reviewed-by:
>>>>
>>>> 6919934 added some unguarded cmov instructions which hit a guarantee
>>>> on older hardware (pre-i686).
>>>>
>>>> The fix is to either guard the added cmov instructions with a
>>>> supports_cmov() or let the cmov instruction decide itself if the
>>>> hardware supports cmov and if not use a branch-mov idiom.  I chose the
>>>> latter.
>>>>
>>>> src/cpu/x86/vm/assembler_x86.cpp
>>>> src/cpu/x86/vm/assembler_x86.hpp
>>>> src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
>>>> src/cpu/x86/vm/c1_Runtime1_x86.cpp
>>>> src/cpu/x86/vm/templateTable_x86_32.cpp
>>
>>
>


More information about the hotspot-compiler-dev mailing list