RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
Civlin, Jan
jan.civlin at intel.com
Wed Oct 21 00:43:29 UTC 2015
Thank you, Vladimir.
-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
Sent: Tuesday, October 20, 2015 5:38 PM
To: Civlin, Jan
Cc: hotspot compiler
Subject: Re: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
Okay, it looks good.
Thanks,
Vladimir
On 10/21/15 12:46 AM, Civlin, Jan wrote:
> Vladimir,
>
> Here is the updated patch with the straight "&& (vopc != Op_CMoveD || vlen == 4);" in VectorNode::implemented.
>
> Please take a look and review.
>
> http://cr.openjdk.java.net/~iveresov/vector-cmove/webrev.03/
>
>
> Thank you,
>
> Jan.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Thursday, October 15, 2015 9:49 PM
> To: Civlin, Jan
> Cc: hotspot compiler; Berg, Michael C
> Subject: Re: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
>
> On 10/16/15 9:12 AM, Civlin, Jan wrote:
>> Vladimir,
>>
>> Michael Berg is about to add this new function for filtering based on vlen, so I use it now for my case and Michael will add more cases soon.
>>
>> const bool Matcher::match_rule_supported_vlen(int opcode, int vlen)
>
> For your changes I would simple add next to the check in
> VectorNode::implemented():
>
> && (vopc != Op_CMoveD || vlen == 4);
>
> For changes which Michael is working on, you have to add it to all platforms .ad (same as match_rule_supported()).
>
> I would suggest to have match_rule_supported_vector(int opcode, int
> vlen) which you will call from VectorNode::implemented() instead of
> match_rule_supported(). And move all current vector opcodes check in
> match_rule_supported() to new one.
>
> Thanks,
> Vladimir
>
>>
>>
>> The updated webrev is here:
>>
>> http://cr.openjdk.java.net/~iveresov/vector-cmove/webrev.02/
>>
>>
>> Thank you,
>>
>> Jan.
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Wednesday, October 14, 2015 2:30 AM
>> To: Civlin, Jan
>> Cc: hotspot compiler
>> Subject: Re: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
>>
>> I said before that condition in .ad file in Matcher::match_rule_supported():
>>
>> case Op_CMoveVD:
>> if (UseAVX > 2) <<<<< (UseAVX < 1 || UseAVX > 2)
>> ret_value = false;
>>
>> should match predicates:
>>
>> predicate(UseAVX > 0 && UseAVX < 3 && n->as_Vector()->length() == 4);
>>
>> Also why only length 4? There is no length 2? I don't see any code in superword.cpp which limits size of CMoveVD vector.
>>
>> Otherwise it seems fine.
>>
>> Thanks,
>> Vladimir
>>
>> On 10/14/15 1:57 AM, Civlin, Jan wrote:
>>> Vladimir,
>>>
>>> the updated webrev is at
>>>
>>> http://cr.openjdk.java.net/~iveresov/vector-cmove/webrev.01/
>>>
>>> Thank you, Igor.
>>>
>>> Best,
>>>
>>> Jan.
>>>
>>> -----Original Message-----
>>> From: Igor Veresov [mailto:igor.veresov at oracle.com]
>>> Sent: Tuesday, October 13, 2015 10:51 AM
>>> To: Civlin, Jan
>>> Cc: Vladimir Kozlov
>>> Subject: Re: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
>>>
>>> Done.
>>>
>>>> On Oct 13, 2015, at 9:51 AM, Civlin, Jan <jan.civlin at intel.com> wrote:
>>>>
>>>> Igor,
>>>>
>>>> I got a response from the moderator that the previous message (attached below) has been rejected due to the size limits.
>>>> Please upload the attached webrev (I guess it will be
>>>> http://cr.openjdk.java.net/~iveresov/vector-cmove/webrev.01/)
>>>>
>>>> And I'll send the link to the group.
>>>>
>>>> Thank you,
>>>>
>>>> Jan.
>>>>
>>>> -----Original Message-----
>>>> From: Civlin, Jan
>>>> Sent: Monday, October 12, 2015 10:19 PM
>>>> To: Vladimir Kozlov
>>>> Cc: Igor Veresov; hotspot compiler; Civlin, Jan
>>>> Subject: RE: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
>>>>
>>>> Vladimir,
>>>>
>>>> Here is a webrev built from a single changeset.
>>>> Please use it for further reviewing.
>>>>
>>>> Thank you,
>>>>
>>>> Jan.
>>>>
>>>> -----Original Message-----
>>>> From: Civlin, Jan
>>>> Sent: Saturday, October 10, 2015 12:14 PM
>>>> To: Vladimir Kozlov
>>>> Cc: hotspot compiler
>>>> Subject: RE: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
>>>>
>>>> Vladimir,
>>>>
>>>> Thank you for the quick review.
>>>>
>>>>
>>>> I thought you are patching your repo using hotspot.patch which is clean of empty files or .hgtag and you use webrev only for facilitation of visual code review.
>>>> If you use hotspot.patch it does not propagate to your repo empty or already integrated in 8136725 files or my repo tags, since none of them are in the patch.
>>>> I thought you are simply skipping unchanged files if you see them in webrev html pages.
>>>>
>>>> My repo is a consistent and long living clone of your repo. It is a single place that includes the history of all my changes, which let me go ahead in development way far of what is ready for going out to you. I can create an additional repo and filter out my tags on way out, but it will create more possibilities for accidental errors.
>>>>
>>>>
>>>> x86.ad: it is indeed not (yet) AVX3, since AVX3 will have different ISA. Once we have AVX3 in the house and can debug it, AVX3 case will be added.
>>>>
>>>> c2_globals.hpp:
>>>> I will rename the flag to UseCMoveUnconditionally, but I see no point of having another flag for the vector: once SuperWord detects CMovD, it merely vectorize it.
>>>> In the opposite direction: one cannot ask for vectorization of CMovD and not to get them in the scalar case.
>>>>
>>>> compile.cpp:
>>>> 1114: will add "do_vector_loop() &&" and change to NOT_PRODUCT(if
>>>> (do_vector_loop() && Verbose&& && has_method())
>>>> {tty->print("Compile::Init: use CMove without profitability tests
>>>> for method %s\n", method()->name()->as_quoted_ascii());})
>>>>
>>>> loopopts.cpp:
>>>> 601: will add "&& cmp_op == Op_CmpD" and change to if
>>>> (C->use_cmove() && cmp_op == Op_CmpD) ;//keep going
>>>>
>>>>
>>>> Thank you,
>>>>
>>>> Jan
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>> Sent: Saturday, October 10, 2015 5:52 AM
>>>> To: Civlin, Jan
>>>> Cc: hotspot compiler
>>>> Subject: Re: RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
>>>>
>>>> Changes contains also 8136725 (loop reserve copy) changes. Please, clean up.
>>>>
>>>> x86.ad WAIT! According to next conditions (BTW, support condition should match predicate) this is only supported with AVX1 and AVX2 and not AVX3. Really?
>>>>
>>>> I think it should be reversed condition (< 3):
>>>> + case Op_CMoveVD:
>>>> + if (UseAVX > 2)
>>>> + ret_value = false;
>>>>
>>>> + instruct vcmov4D_reg(vecY dst, vecY src1, vecY src2, immI8 cop,
>>>> cmpOp_vcmppd copnd) %{
>>>> + predicate(UseAVX > 0 && UseAVX < 3 && n->as_Vector()->length()
>>>> + == 4);
>>>>
>>>>
>>>> x86_64.ad changes are empty (could be only spacing change) so remove it from change.
>>>>
>>>> c2_globals.hpp DoReserveCopyInSuperWord is defined in 8136725 changes.
>>>> UseCMov should be UseCMoveUnconditionally I think (note 'move'
>>>> instead of 'mov'). Also I am not sure that you should mix 2 things
>>>> into this
>>>> changes: one is vectorizing cmoveD (for doubles) and always generate cmoves. I think it is different issues and this change should concentrate on vectorizing only double cmoves. Then flag could be VectorizeCMoveD.
>>>>
>>>> compile.cpp print not under UseCmov flag:
>>>> + NOT_PRODUCT(if (Verbose && has_method()) {tty->print("Compile::Init:
>>>> use CMove without profitability tests for method %s\n",
>>>> method()->name()->as_quoted_ascii());})
>>>>
>>>> loopopts.cpp changes inconsistent. In one place you check use_cmove only for doubles and in other for all.
>>>>
>>>> I will look on superword changes after you cleanup changes: remove
>>>> 8136725 and also create history from fresh repo - I don't want to see 50 revisions descriptions in webrev.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 10/10/15 7:04 AM, Civlin, Jan wrote:
>>>>> Please review this patch.
>>>>>
>>>>> Hi All,
>>>>>
>>>>>
>>>>> We would like to contribute the SuperWord enhancement to
>>>>> support vector conditional move (CMovVD ) on Intel AVX cpu.
>>>>>
>>>>>
>>>>> The contribution Bug ID: 8139340.
>>>>>
>>>>> Please review this patch:
>>>>>
>>>>> Bug-id: https://bugs.openjdk.java.net/browse/JDK-8139340
>>>>>
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~iveresov/vector-cmove/webrev.00/
>>>>>
>>>>> Description:
>>>>>
>>>>> SuperWord enhancement to support vector conditional move (CMovVD)
>>>>> on Intel AVX cpu.
>>>>>
>>>>> The SuperWord optimization bails out on counted loops that contain
>>>>> any conditional statement other than the loop exit, and this
>>>>> prevents vectorization of many compute bound loops.
>>>>>
>>>>> The proposed enhancement enables generation of CMovD on demand
>>>>> (-XX:+UseCMov), and further vectorization of CMovD (into CMovVD )
>>>>> in SuperWord optimization.
>>>>>
>>>>> The performance gain observed on a simplified Monte Carlo Option
>>>>> Calculation was up to 2x speed-up.
>>>>>
>>>>> Thank you,
>>>>>
>>>>> Jan.
>>>>>
>>>>> *From:*Igor Veresov [mailto:igor.veresov at oracle.com]
>>>>> *Sent:* Friday, October 9, 2015 3:22 PM
>>>>> *To:* Civlin, Jan
>>>>> *Cc:* hotspot compiler; Vladimir Kozlov
>>>>> *Subject:* Re: SuperWord enhancement to support vector conditional
>>>>> move (CMovVD ) on Intel AVX cpu.
>>>>>
>>>>> Here it is: https://bugs.openjdk.java.net/browse/JDK-8139340
>>>>>
>>>>> igor
>>>>>
>>>>> On Oct 9, 2015, at 2:57 PM, Civlin, Jan <jan.civlin at intel.com
>>>>> <mailto:jan.civlin at intel.com>> wrote:
>>>>>
>>>>> Thank you, Igor.
>>>>>
>>>>> What is the RFR for this?
>>>>>
>>>>> *From:*Igor Veresov [mailto:igor.veresov at oracle.com]
>>>>> *Sent:*Friday, October 9, 2015 2:53 PM
>>>>> *To:*Civlin, Jan
>>>>> *Cc:*hotspot compiler; Vladimir Kozlov
>>>>> *Subject:*Re: SuperWord enhancement to support vector conditional
>>>>> move (CMovVD ) on Intel AVX cpu.
>>>>>
>>>>> Here the webrev:
>>>>> http://cr.openjdk.java.net/~iveresov/vector-cmove/webrev.00/
>>>>>
>>>>> igor
>>>>>
>>>>> On Oct 9, 2015, at 1:15 PM, Civlin, Jan <jan.civlin at intel.com
>>>>> <mailto:jan.civlin at intel.com>> wrote:
>>>>>
>>>>> Igor,
>>>>>
>>>>> Please create RFR and upload this patch. You may need to rename
>>>>> ancnav.js.remove_this_extention back to ancnav.js (I have to
>>>>> rename it for passing the mail server filters).
>>>>>
>>>>> Description:
>>>>>
>>>>> SuperWord enhancement to support vector conditional move
>>>>> (CMovVD) on Intel AVX cpu.
>>>>>
>>>>> The SuperWord optimization bails out on counted loops that
>>>>> contain any conditional statement other than the loop exit, and
>>>>> this prevents vectorization of many compute bound loops.
>>>>>
>>>>> The proposed enhancement enables generation of CMovD on demand
>>>>> (-XX:+UseCMov), and further vectorization of CMovD (into CMovVD
>>>>> ) in SuperWord optimization.
>>>>>
>>>>> The performance gain observed on a simplified Monte Carlo Option
>>>>> Calculation was up to 2x speed-up.
>>>>>
>>>>> Thank you,
>>>>>
>>>>> Jan.
>>>>>
>>>>> <webrev-r9162-9157.tar.bz2>
>>>>>
>>>> <webrev-r9050.tar.bz2>
>>>
More information about the hotspot-compiler-dev
mailing list