RFR: 8139340: SuperWord enhancement to support vector conditional move (CMovVD ) on Intel AVX cpu.
Berg, Michael C
michael.c.berg at intel.com
Fri Oct 16 18:36:29 UTC 2015
Initially I thought about doing that very thing. I would take this in different direction though, thus avoiding two duplicate lists. That is to have match_rule_supported_vector call match_rule_supported, keeping the lists disjunct. The vector length constraints are fewer in number and divergent and we don't' want people mixing the context rules as they have different meanings. It also removes the necessity of maintaining two identical lists. Otherwise granularity of control could become harder to manage.
Thanks,
Michael
-----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