RE: r262817 - [CLANG][AVX512][BUILTIN] Adding vpmultishiftqb{128|256|512}

2016-04-12 Thread Demikhovsky, Elena via cfe-commits 
Yes, we’ll try make the patches smaller and avoid such mistakes.

-   Elena

From: Chandler Carruth [mailto:chandl...@google.com]
Sent: Tuesday, April 12, 2016 20:25
To: Zuckerman, Michael ; 
cfe-commits@lists.llvm.org; Demikhovsky, Elena 
Subject: Re: r262817 - [CLANG][AVX512][BUILTIN] Adding 
vpmultishiftqb{128|256|512}

On Mon, Mar 7, 2016 at 12:33 AM Michael Zuckerman via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: mzuckerm
Date: Mon Mar  7 02:29:10 2016
New Revision: 262817

URL: http://llvm.org/viewvc/llvm-project?rev=262817&view=rev
Log:
[CLANG][AVX512][BUILTIN] Adding vpmultishiftqb{128|256|512}

This commit log seems super confusing combined with:

+TARGET_BUILTIN(__builtin_ia32_pbroadcastb512_gpr_mask, 
"V64ccV64cULLi","","avx512bw")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastb128_gpr_mask, 
"V16ccV16cUs","","avx512bw,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastb256_gpr_mask, 
"V32ccV32cUi","","avx512bw,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastd128_gpr_mask, 
"V4iiV4iUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastd256_gpr_mask, 
"V8iiV8iUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastq128_gpr_mask, 
"V2LLiULLiV2LLiUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastq256_gpr_mask, 
"V4LLiULLiV4LLiUc","","avx512vl")

This is adding *broadcast* intrinsics!!! Not multishift!

Please be much more careful with your patch descriptions. I just happened to 
randomly notice this.

Elena, please actually carefully review these patches.
-
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r274110 - [AVX512] Zero extend cmp intrinsic return value.

2016-07-04 Thread Demikhovsky, Elena via cfe-commits 
I think that AVX-512 intrinsic handled in Clang should be removed from 
AutoUpgrade.
If we decide to replace a call to llvm-backend intrinsic with generating LLVM 
IR directly from clang, we can remove llvm-backend intrinsic at all.
Otherwise we’ll need to duplicate support in FE and BE for all AVX-512 
intrinsics.

-   Elena

From: Craig Topper [mailto:craig.top...@gmail.com]
Sent: Monday, July 04, 2016 21:53
To: Breger, Igor 
Cc: Eric Christopher via cfe-commits ; Demikhovsky, 
Elena 
Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

We have to keep the autoupgrade code due to the requirement that we need to be 
able to load bitcode files from previous versions of llvm. Though the intrinsic 
autograde code is starting to become large.

~Craig

On Mon, Jul 4, 2016 at 2:55 AM, Breger, Igor 
mailto:igor.bre...@intel.com>> wrote:
Thanks for working on this!

Regards,
Igor

From: Craig Topper 
[mailto:craig.top...@gmail.com]
Sent: Monday, July 04, 2016 10:20
To: Breger, Igor
Cc: Eric Christopher via cfe-commits; Demikhovsky, Elena; Ouriel, Boaz
Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

I've modified the indices we use for the zero vector in r274484. This gives 
better codegen as it now looks like a concat to the backend. So now we just end 
up emitting unnecessary kshiftlw/kshiftrw pairs instead of converting to a 
wider vector op and back.

~Craig

On Sun, Jul 3, 2016 at 1:32 PM, Craig Topper 
mailto:craig.top...@gmail.com>> wrote:
Also should we change the AutoUpgrade code for the cmp intrinsics in the 
backend to also use zero instead of undef?

~Craig

On Sun, Jul 3, 2016 at 12:11 AM, Breger, Igor 
mailto:igor.bre...@intel.com>> wrote:
Hello Craig,
Thanks a lot for pointing it out to me.  I familiar with this problem,  we are 
planning  to improve CodeGen to handle with this case in near future.

Regards,
Igor

From: Craig Topper 
[mailto:craig.top...@gmail.com]
Sent: Saturday, July 02, 2016 08:46
To: Breger, Igor; Eric Christopher via cfe-commits
Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

This change codgens to something really awful now. Can you take a look?

.section__TEXT,__text,regular,pure_instructions
.section__TEXT,__literal8,8byte_literals
.p2align   3
LCPI0_0:
.quad   -1
.section__TEXT,__const
.p2align   6
LCPI0_1:
.quad   0
.quad   1
.quad   2
.quad   3
.quad   8
.quad   8
.quad   8
.quad   8
.section__TEXT,__text,regular,pure_instructions
.globl   _test_mm_cmpeq_epu32_mask
.p2align   4, 0x90
_test_mm_cmpeq_epu32_mask:
vpcmpeqd   %xmm1, %xmm0, %k1
vpbroadcastq   LCPI0_0(%rip), %zmm0 {%k1} {z}
vpxord %zmm1, %zmm1, %zmm1
vmovdqa64 LCPI0_1(%rip), %zmm2
vpermt2q %zmm1, %zmm2, %zmm0
vpsllq   $63, %zmm0, %zmm0
vptestmq %zmm0, %zmm0, %k0
kmovw%k0, %eax
retq

~Craig

On Wed, Jun 29, 2016 at 1:14 AM, Igor Breger via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: ibreger
Date: Wed Jun 29 03:14:17 2016
New Revision: 274110

URL: http://llvm.org/viewvc/llvm-project?rev=274110&view=rev
Log:
[AVX512]  Zero extend cmp intrinsic return value.

Differential Revision: http://reviews.llvm.org/D21746

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/avx512vl-builtins.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=274110&r1=274109&r2=274110&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jun 29 03:14:17 2016
@@ -6460,8 +6460,8 @@ static Value *EmitX86MaskedCompare(CodeG
   Indices[i] = i;
 for (unsigned i = NumElts; i != 8; ++i)
   Indices[i] = NumElts;
-Cmp = CGF.Builder.CreateShuffleVector(Cmp, UndefValue::get(Cmp->getType()),
-  Indices);
+Cmp = CGF.Builder.CreateShuffleVector(
+Cmp, llvm::Constant::getNullValue(Cmp->getType()), Indices);
   }
   return CGF.Builder.CreateBitCast(Cmp,
IntegerType::get(CGF.getLLVMContext(),

Modified: cfe/trunk/test/CodeGen/avx512vl-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512vl-builtins.c?rev=274110&r1=274109&r2=274110&view=diff
==
--- cfe/trunk/test/CodeGen/avx512vl-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512vl-builtins.c Wed Jun

RE: r274110 - [AVX512] Zero extend cmp intrinsic return value.

2016-07-05 Thread Demikhovsky, Elena via cfe-commits 
Most of AVX-512 intrinsics were not supported in 3.8 version and added between 
3.8 and 3.9.
In this case, when a BE intrinsic was introduced between releases, it can be 
removed, right?

Anyway, we’d better keep the BE intrinsic inside than maintain it in the 
autoupgrade.

> Also should we change the AutoUpgrade code for the cmp intrinsics in the 
> backend to also use zero instead of undef?
This pass was initially designed in order to keep backward compatibility and 
not for optimizations, right?

-   Elena

From: Craig Topper [mailto:craig.top...@gmail.com]
Sent: Tuesday, July 05, 2016 09:39
To: Demikhovsky, Elena 
Cc: Breger, Igor ; Eric Christopher via cfe-commits 
; LLVM Commits (llvm-comm...@lists.llvm.org) 

Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

Unfortunately the policy is such that the current version of LLVM must be able 
to read a bitcode or IR text file generated by any released version of 
LLVM/clang from 3.0 till now. This is why the AutoUpgrade.cpp file exists and 
it keeps growing and its only going to get worse.

Hopefully with the discussion about the version number for the release after 
3.9 we end up with a time based compatibility policy. This will allow us to 
remove the auto upgrade code over time, but it will take many years before we 
can remove auto upgrade being added now.

On Monday, July 4, 2016, Demikhovsky, Elena 
mailto:elena.demikhov...@intel.com>> wrote:
I think that AVX-512 intrinsic handled in Clang should be removed from 
AutoUpgrade.
If we decide to replace a call to llvm-backend intrinsic with generating LLVM 
IR directly from clang, we can remove llvm-backend intrinsic at all.
Otherwise we’ll need to duplicate support in FE and BE for all AVX-512 
intrinsics.

-   Elena

From: Craig Topper 
[mailto:craig.top...@gmail.com]
Sent: Monday, July 04, 2016 21:53
To: Breger, Igor 
>
Cc: Eric Christopher via cfe-commits 
>;
 Demikhovsky, Elena 
>
Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

We have to keep the autoupgrade code due to the requirement that we need to be 
able to load bitcode files from previous versions of llvm. Though the intrinsic 
autograde code is starting to become large.

~Craig

On Mon, Jul 4, 2016 at 2:55 AM, Breger, Igor 
> 
wrote:
Thanks for working on this!

Regards,
Igor

From: Craig Topper 
[mailto:craig.top...@gmail.com]
Sent: Monday, July 04, 2016 10:20
To: Breger, Igor
Cc: Eric Christopher via cfe-commits; Demikhovsky, Elena; Ouriel, Boaz
Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

I've modified the indices we use for the zero vector in r274484. This gives 
better codegen as it now looks like a concat to the backend. So now we just end 
up emitting unnecessary kshiftlw/kshiftrw pairs instead of converting to a 
wider vector op and back.

~Craig

On Sun, Jul 3, 2016 at 1:32 PM, Craig Topper 
>
 wrote:
Also should we change the AutoUpgrade code for the cmp intrinsics in the 
backend to also use zero instead of undef?

~Craig

On Sun, Jul 3, 2016 at 12:11 AM, Breger, Igor 
> 
wrote:
Hello Craig,
Thanks a lot for pointing it out to me.  I familiar with this problem,  we are 
planning  to improve CodeGen to handle with this case in near future.

Regards,
Igor

From: Craig Topper 
[mailto:craig.top...@gmail.com]
Sent: Saturday, July 02, 2016 08:46
To: Breger, Igor; Eric Christopher via cfe-commits
Subject: Re: r274110 - [AVX512] Zero extend cmp intrinsic return value.

This change codgens to something really awful now. Can you take a look?

.section__TEXT,__text,regular,pure_instructions
.section__TEXT,__literal8,8byte_literals
.p2align   3
LCPI0_0:
.quad   -1
.section__TEXT,__const
.p2align   6
LCPI0_1:
.quad   0
.quad   1
.quad   2
.quad   3
.quad   8
.quad   8
.quad   8
.quad   8
.section__TEXT,__text,regular,pure_instructions
.globl   _test_mm_cmpeq_epu32_mask
.p2align   4, 0x90
_test_mm_cmpeq_epu32_mask:
vpcmpeqd   %xmm1, %xmm0, %k1
vpbroadcastq   LCPI0_0(%rip), %zmm0 {%k1} {z}
vpxord %zmm1, %zmm1, %zmm1
vmovdqa64 LCPI0_1(%rip), %zmm2
vpermt2q %zmm1, %zmm2, %zmm0
vpsllq   $63, %zmm0, %zmm0
vptestmq %zmm0, %zmm0, %k0
kmovw%k0, %eax
retq

~Craig

On Wed, Jun 29, 2016 at 1:14 AM, Igor Breger via cfe-commits 
>
 wrote:
Author: ibreger
Date: Wed Jun 29 03:14:17 2016
New Revision: 274110

URL: http://llvm.org/viewvc/llvm-project?rev=274110&view=rev
Log:
[AVX512]  Zero extend cmp intrinsic return value.

Differential Revision: http://reviews.llvm.org/D21746

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/