Request for reviews (XL): 7116452: Add support for AVX instructions

Vladimir Kozlov vladimir.kozlov at oracle.com
Wed Dec 14 12:54:26 PST 2011


Tom Rodriguez wrote:
> Overall it looks good.
> 
> Are there any hidden uses of rscratch1 that's we've introduced?  I was looking at d2l_fixup and the current code always explicitly assumes that double_sign_flip is reachable but the new code uses cmp64 which might use rscratch1.  It won't because double_sign_flip is allocated from the CodeCache but it had me worried for a minute.  We should have a flag for MacroAssembler that disables the implicit use of rscratch1 so we don't get silently screwed.

It is used in all float instructions which have constant operand (so it is 
reachable). And I can't use cmpq instead of cmp64 there since as_Address() 
method can be called only from MacroAssembler. But I agree with you that if we 
use assembler instruction in ad file we should have only reachable addresses. 
Something like this:

void MacroAssembler::addss(XMMRegister dst, AddressLiteral src, bool 
only_reachable) {
   if (reachable(src)) {
     addss(dst, as_Address(src));
   } else {
     assert(!only_reachable, "");
     lea(rscratch1, src);
     addss(dst, Address(rscratch1, 0));
   }
}

> 
> In assembler_x86.cpp could you put a small comment on simd_pre and simd_opc?

Done:

+ // SSE SIMD prefix byte values corresponding to VexSimdPrefix encoding.
   static int simd_pre[4] = { 0, 0x66, 0xF3, 0xF2 };
+ // SSE opcode second byte values (first is 0x0F) corresponding to VexOpcode 
encoding.
   static int simd_opc[4] = { 0,    0, 0x38, 0x3A };

+ // Generate SSE legacy REX prefix and SIMD opcode based on VEX encoding.
   void Assembler::rex_prefix(Address adr, XMMRegister xreg, VexSimdPrefix pre, 
VexOpcode opc, bool rex_w) {

> 
> Reading through the x86 ad files really put me in the mood to eliminate the spurious formatting and naming differences between them.  Large portions of them are (or could be) exactly the same.

I have the same feeling. I want to create third x86.ad file which has common 
instructions definitions. Currently Xmm registers naming is different (regXD vs 
regD) but we can fix it.

Thanks,
Vladimir

> 
> tom
> 
> On Dec 8, 2011, at 3:53 PM, Vladimir Kozlov wrote:
> 
>> http://cr.openjdk.java.net/~kvn/7116452/webrev.01
>>
>> 7116452: Add support for AVX instructions
>>
>> Initial changes were submitted by Intel. I refactored it to simplify prefix usage in instructions codding (added simd_prefix methods) and VEX encoding was fixed to generate 2bytes prefix when possible. Changes in .ad files were not complete (especially in 32-bit .ad) and were not aggressive as I want. I changed more mach nodes encoding to use macroassembler instructions. Added missing decoding parts in Assembler::locate_operand() and NativeMovRegMem::instruction_start().
>>
>> Note: no new AVX instructions were added in these changes. And no 3 operands format was added to MacroAssembler. It will be other changes. Destination operand is used as second source in current implementation where applicable.
>>
>> Float compare implementation in x86_32.ad was replaced with implementation from x86_64.ad. It uses less branches and does not destroy EAX register. Note: ucomiss instruction produces the same result as comiss since we masking numeric exceptions. Also ucomiss could be a little faster since it does not need to check control word for QNaN values.
>>
>> Vector instructions with VEX prefix use unaligned load for memory operands where with old REX prefix it require 16 bytes alignment. Instructions version with memory operand were added for that but they should be used only with VEX prefix, assert was added. ANDPD and XORPD with memory operand were used before with 16 bytes aligned memory (we have special code to do it). I added assert to check address alignment for these instructions.
>>
>> As part of these changes REX.W prefix was removed from instructions where it was not needed: MOVDQA, MOVDQU, PCMPESTRI, PSRLQ, PSRLDQ, PTEST.
>>
>>
>> Tested with UseAVX=1|0, UseSSE=4|2|1|0, CTW, VM regression tests, nsk.
>>
>> Thanks,
>> Vladimir
> 


More information about the hotspot-compiler-dev mailing list