8038953,Add sanity tests for BMI1 and LZCNT instructions

[Vladimir Kozlov]

Hi Anton,

Did you update webrev?

On 4/7/14 7:14 AM, Anton Ivanov wrote:
> here is new webrev:
> http://cr.openjdk.java.net/~iignatyev/aaivanov/8038953/webrev.02
>
> 1. now tests is executed only if needed and skipped if any of
> requirement is not met

It is good.

> 2. countCpuInstructions() moved to base class

It still abstruct in base class:

protected abstract int countCpuInstructions(byte[] nativeCode);

> 3. 0b001_11000 was a typo, thank you

BlsmskTestI.java still have 0b001_11000.

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
>>>>>>>>
>>>>>
>>>
>