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

[Christian Thalinger]

Thank you, Tom, Igor, Vladimir and Paul.  -- Christian

On Mar 30, 2011, at 11:22 PM, Paul Hohensee wrote:
> 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