8038953,Add sanity tests for BMI1 and LZCNT instructions

[Anton Ivanov]

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
2. countCpuInstructions() moved to base class
3. 0b001_11000 was a typo, thank you


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