On Mon, 21 Sep 2020 09:20:56 GMT, Andrew Dinn <ad...@openjdk.org> wrote:
>>> Can you explain where this restricted effect is documented? >> >> Certainly! I’ve found that determining the capability of the CPU and whether >> to enable AVX2 support if the chip >> supports it is mostly controlled in: [vm_version_x86.cpp]( >> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp) >> specifically: >> [get_processor_features](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L684-L755) >> and in [generate_get_cpu_info]( >> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L69-L611). >> In order to test the >> patch comprehensively I had to track down an Intel Core i7 (I7-9750H) >> processor which the aforementioned code permitted >> AVX2 instructions for (maybe this is an error and it should not be enabled >> for this processor though) as most of the >> infrastructure I personally use here at AWS runs on Intel Xeon processors - >> I also tested on a E5-2680 which the JVM >> does not enable AVX2 for. However, this is just the Intel side of things. >> When it comes to AMD I read that the AMD Zen >> 2 architecture, of which the current flagship: Threadripper 3990X, is based, >> is able to support AVX2 [without the >> frequency scaling]( >> https://www.anandtech.com/Show/Index/14525?cPage=7&all=False&sort=0&page=9&slug=amd-zen-2-microarchitecture-analysis-ryzen-3000-and-epyc-rome) >> which some/all(?) of the Intel chips incur. I personally don’t have access >> to one of these chips so I cannot confirm >> how it is classified in the JVM. Also, I found when investigating this that >> there is actually a JVM flag which can be >> used to control what level of AVX is enabled: `-XX:UseAVX=version.` > >> > Can you explain where this restricted effect is documented? > >> Certainly! I’ve found that determining the capability of the CPU and whether >> to enable AVX2 support if the chip >> supports it is mostly controlled in: vm_version_x86.cpp specifically: >> get_processor_features and in >> generate_get_cpu_info. > > Yes, I can see what the code does. I was asking where the cpu behaviour is > documented independent of the code. > >> In order to test the patch comprehensively I had to track down an Intel Core >> i7 (I7-9750H) processor which the >> aforementioned code permitted AVX2 instructions for (maybe this is an error >> and it should not be enabled for this >> processor though) as most of the infrastructure I personally use here at AWS >> runs on Intel Xeon processors - I also >> tested on a E5-2680 which the JVM does not enable AVX2 for. > > 'maybe'? The documentation Andrew provided mentioned Xeon E5 v3 which I > believe is a Skylake design. However, the code > I pointed you at in vm_version_x86 which claims to detect 'early Skylake' > designs is only disabling AVX512 support. It > still enables AVX2. Similarly, the code that generates machine code to check > the processor capabilities has a special > check if use_evex is set (i.e. AVX3 is requested) which disables AVX512 for > Skylake but does not disable AVX2 support. >> However, this is just the Intel side of things. When it comes to AMD I read >> that the AMD Zen 2 architecture, of which >> the current flagship: Threadripper 3990X, is based, is able to support AVX2 >> without the frequency scaling which >> some/all(?) of the Intel chips incur. I personally don’t have access to one >> of these chips so I cannot confirm how it >> is classified in the JVM. > > Well, it would be good to know where you read that and to see if that > confirms thar the code is avoiding the issue > Andrew raised. >> Also, I found when investigating this that there is actually a JVM flag >> which can be used to control what level of AVX >> is enabled: -XX:UseAVX=version. > > Yes, indeed. However, what I am trying to understand is whether the current > code is bypassing the problem Andrew > brought up in the cases where that problem actually exists. It doesn't look > like it so far given that the problem > applies to AVX2 and only AVX512 support is being disabled and, even then only > for some (Skylake) processors. Without > some clear documentation of what processors suffer from this power surge > problem it will not be possible to decide > whether this patch is doing the right thing or not. Based on comment by @jatin-bhateja (Intel) frequency level switchover pointed by @theRealAph is sensitive to vector size https://github.com/openjdk/jdk/pull/144#issuecomment-692044896 By keeping vector size less or equal to 32 bytes we should avoid it. And as I can see this intrinsic code is using 32 bytes (chars) and 16 bytes vectors: `pbroadcastb(vec1, vec1, Assembler::AVX_256bit);` Also we never had issues with AVX2. only with AVX512 regarding performance hit: https://bugs.openjdk.java.net/browse/JDK-8221092 I would like to see performance numbers for for all values of UseAVX flag : 0, 1, 2, 3 The usage is guarded UseSSE42Intrinsics in UseSSE42Intrinsics predicate in .ad file. Make sure to test with UseAVX=0 to make sure that some avx instructions are not mixed into non avx code. And also with UseSSE=2 (for example) to make sure shared code correctly recognize that intrinsics is not supported. ------------- PR: https://git.openjdk.java.net/jdk/pull/71
