8038953,Add sanity tests for BMI1 and LZCNT instructions

[Vladimir Kozlov]

In BlsiTestI.java, BlsmskTestI.java, BlsrTestI.java,  you check 5 
elements but the scanned size is length - 4.

On 4/8/14 3:30 AM, Anton Ivanov wrote:
>
>>> 2. countCpuInstructions() moved to base class
>> It still abstruct in base class:
>
> countCpuInstructions() should be abstract. Concrete classes has their's
> own implementation depending on what instruction they are searching for,
> there is no sense to make this method non-abstract, it should be
> overwritten in every ancestor.

Since countCpuInstructions have use the same code I asked for something 
like next in based class:

     protected int countCpuInstructions(byte[] nativeCode, int codeSize, 
byte[] mask, byte[] opcode) {
         int n = 0;
         for (int i = 0, size = nativeCode, length - codeSize; i < size; 
i++) {
             int j = 0;
             for (; j < codeSize; j++) {
                 if (nativeCode[i + j] & mask[j]) != opcode[j]) {
                     break;
                 }
             }
             if (j == codeSize) {
                 n++;
             }
         }
         return n;
      }

For matching whole byte you use 0xFF mask. For bytes you don't want to 
compare mask = 0 and opcode = 0.

Regards,
Vladimir

>
>> protected abstract int countCpuInstructions(byte[] nativeCode);
>>> 3. 0b001_11000 was a typo, thank you
>> BlsmskTestI.java still have 0b001_11000.
> fixed
>
> webrev: http://cr.openjdk.java.net/~iignatyev/aaivanov/8038953/webrev.03
> <http://cr.openjdk.java.net/%7Eiignatyev/aaivanov/8038953/webrev.03>
>
>
>> Thanks,
>> Vladimir
>>
>>>
>>>
>>> On 03.04.2014 22:33, Vladimir Kozlov wrote:
>>>> On 4/3/14 8:09 AM, Anton Ivanov wrote:
>>>>> Here is the new webrev
>>>>> http://cr.openjdk.java.net/~iignatyev/aaivanov/8038953/webrev.01
>>>>> tests directory renamed to verifycode
>>>>> skipping on not-target platform
>>>>
>>>> Why not to skip when !isIntrinsicSupported()? Otherwise you will get
>>>> very long useless testing output and at the end:
>>>> Java HotSpot(TM) 64-Bit Server VM warning: lzcnt instruction is not
>>>> available on this CPU
>>>>
>>>> Just try to run on CPU which does not support this instructions.
>>>>
>>>> Do you plan to test Client VM (C1) in a future? If not we should skip
>>>> whole testing if it is not Platform.isServer().
>>>>
>>>> May be report in checkEmittedCode() when we don't check instructions
>>>> to know that that test passed without check.
>>>> Also in checkEmittedCode() method verifyPositive() should be called
>>>> only in scope where its result is used.
>>>>
>>>> We always get complain that tests run too long and your tests execute
>>>> a lot of code with not used result.
>>>>
>>>> The check code in countCpuInstructions() looks the same and can be
>>>> moved into common method.
>>>>
>>>> I see in one case you use 0b0011_1000 and in an other 0b001_11000,
>>>> typo?
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>> negative check (for "Unexpected CPU instructions found") removed
>>>>>
>>>>>
>>>>> On 01.04.2014 23:56, Vladimir Kozlov wrote:
>>>>>> Okay, I see that you do use code from Filipp's changes (for example,
>>>>>> TestAndnI.AndnIExpr).
>>>>>>
>>>>>> I think subdir name 'sanity' is not accurate and too general.
>>>>>> Something like 'verifycode' or similar is more reasonable.
>>>>>>
>>>>>> Why you not bailing out (skip test) in main() (or in test()) when
>>>>>> intrinsic is not used?
>>>>>>
>>>>>> An asm instruction could be used without intrinsic. It all depends on
>>>>>> mach instructions matching rules in implementation in .ad file. So
>>>>>> your  AssertionError "Unexpected CPU instructions found: " could be
>>>>>> triggered. Also what if C1 also generate such code? I don't know why
>>>>>> you have this assert.
>>>>>>
>>>>>> I think you need to add WB API to get info if a particular intrinsic
>>>>>> was really used. Even if you pass UseCountTrailingZerosInstruction
>>>>>> flag it could be overwritten by InlineNatives flag. There could be
>>>>>> other legit reasons we don't use an intrinsic.
>>>>>>
>>>>>> On 4/1/14 10:51 AM, Anton Ivanov wrote:
>>>>>>> Hi Vladimir,
>>>>>>>
>>>>>>> They test different things,
>>>>>>> Fillip's tests - verify results to be the same for compiler and
>>>>>>> interpreter (but imagine compiler do not use intrinsic at all, test
>>>>>>> will
>>>>>>> pass too, because they will do exactly the same operations)
>>>>>>
>>>>>> It is not true. Even when it is not intrinsic JIT still generates
>>>>>> assembler code which could be problematic.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>>> My tests verify only that certain asm instructions are presented in
>>>>>>> native code emitted by compiler (but do not verify the
>>>>>>> correctness of
>>>>>>> execution)
>>>>>>> Combination of these 2 sets of tests makes sure that new
>>>>>>> instructions
>>>>>>> are really used, and used correctly.
>>>>>>> It also was not easy to modify Fillip's tests because they were
>>>>>>> developed in parallel
>>>>>>>
>>>>>>>
>>>>>>> combination of these 2 sets give us
>>>>>>>
>>>>>>> On 01.04.2014 21:03, Vladimir Kozlov wrote:
>>>>>>>> I mean why do you need to create separate tests and not use (add
>>>>>>>> new
>>>>>>>> code) existing one?
>>>>>>>> For example, you added bmi/sanity/AddnTestI.java when there is
>>>>>>>> already
>>>>>>>> bmi/TestAndnI.java.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> On 4/1/14 9:57 AM, Vladimir Kozlov wrote:
>>>>>>>>> Anton,
>>>>>>>>>
>>>>>>>>> How your changes relate to Filipp's?:
>>>>>>>>>
>>>>>>>>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/15d507abfc7a
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>>>
>>>>>>>>> On 4/1/14 9:40 AM, Anton Ivanov wrote:
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> Please review the patch
>>>>>>>>>>
>>>>>>>>>> New tests for using of blsi,blsr,andn,blsmsk,lzcnt,tzcnt
>>>>>>>>>> instructions were developed.
>>>>>>>>>> They verify that instructions above are really presented in
>>>>>>>>>> emitted
>>>>>>>>>> native code on supported hardware
>>>>>>>>>>
>>>>>>>>>> webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~iignatyev/aaivanov/8038953/webrev.00
>>>>>>>>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8038953
>>>>>>>>>> testing: test/compiler/intrinsics/bmi/sanity
>>>>>>>>>>
>>>>>>>
>>>>>
>>>
>
> --
> Best regards,
> Anton Ivanov
>