[X86][AVX512] Removing llvm x86 intrinsics for _mm_mask_move_{ss|sd} intrinsics.

[aymanmus]

Summary

Removing the intrinsics as they are no longer needed. Clang now lowers the C intrinsics to generic llvm IR.
Also updating the AutoUpgrade pass to deal with the removal of these intrinsics.

[delena]

lib/IR/AutoUpgrade.cpp

681

All variables should start from a capital letter.

[aymanmus]

Variables to start with capital letter.

[igorb]

LGTM

[craig.topper]

Two minor comments. But LGTM otherwise.

lib/IR/AutoUpgrade.cpp

341

This is the whole name of the intrinsic so startswith is unnecessary. You should be able to combine both checks to a single Name.startswith("avx512.mask.move.s").

[aymanmus]

This change has been reverted due to failure in build.
compilers do not accept nesting levels greater that 127, and the file AutoUpgrade.cpp contains a long sequence of "if else" statements which the compiler considers too deep.

The error I got:
"FAILED: lib/IR/CMakeFiles/LLVMCore.dir/AutoUpgrade.cpp.obj
C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\lib\IR\AutoUpgrade.cpp(1529): fatal error C1061: compiler limit: blocks nested too deeply"

A heavy refactor to this piece of code should be considered as an effort to reduce the use of llvm x86 intrinsics is being initiated, and as a result many new cases will be added to the same control flow in the future.

[aymanmus]

After commit that reduced the number of "if else" statements (r286768) this patch was recommitted as r287087.

include/llvm/IR/IntrinsicsX86.td

let TargetPrefix = "x86" in {  // All intrinsics start with "llvm.x86.".
   def int_x86_avx_maskload_pd_256 : GCCBuiltin<"__builtin_ia32_maskloadpd256">,
         Intrinsic<[llvm_v4f64_ty], [llvm_ptr_ty, llvm_v4i64_ty],
                   [IntrReadMem, IntrArgMemOnly]>;
   def int_x86_avx_maskload_ps_256 : GCCBuiltin<"__builtin_ia32_maskloadps256">,
         Intrinsic<[llvm_v8f32_ty], [llvm_ptr_ty, llvm_v8i32_ty],
                   [IntrReadMem, IntrArgMemOnly]>;
 }
 
-// Conditional move ops
-let TargetPrefix = "x86" in {  // All intrinsics start with "llvm.x86.".
-  def int_x86_avx512_mask_move_ss :
-        GCCBuiltin<"__builtin_ia32_movss_mask">,
-          Intrinsic<[llvm_v4f32_ty],
-                    [llvm_v4f32_ty, llvm_v4f32_ty, llvm_v4f32_ty, llvm_i8_ty],
-                    [IntrNoMem]>;
-  def int_x86_avx512_mask_move_sd :
-        GCCBuiltin<"__builtin_ia32_movsd_mask">,
-          Intrinsic<[llvm_v2f64_ty],
-                    [llvm_v2f64_ty, llvm_v2f64_ty, llvm_v2f64_ty, llvm_i8_ty],
-                    [IntrNoMem]>;
-}
-
 // Conditional store ops
 let TargetPrefix = "x86" in {  // All intrinsics start with "llvm.x86.".
   def int_x86_avx_maskstore_pd : GCCBuiltin<"__builtin_ia32_maskstorepd">,
         Intrinsic<[], [llvm_ptr_ty,
                   llvm_v2i64_ty, llvm_v2f64_ty], [IntrArgMemOnly]>;
   def int_x86_avx_maskstore_ps : GCCBuiltin<"__builtin_ia32_maskstoreps">,
         Intrinsic<[], [llvm_ptr_ty,
                   llvm_v4i32_ty, llvm_v4f32_ty], [IntrArgMemOnly]>;

lib/IR/AutoUpgrade.cpp

if (IsX86 &&
          Name.startswith("avx512.psrl.dq") ||
          Name == "sse41.pblendw" ||
          Name.startswith("sse41.blendp") ||
          Name.startswith("avx.blend.p") ||
          Name == "avx2.pblendw" ||
          Name.startswith("avx2.pblendd.") ||
          Name.startswith("avx.vbroadcastf128") ||
          Name == "avx2.vbroadcasti128" ||
+         Name.startswith("avx512.mask.move.ss") ||
+         Name.startswith("avx512.mask.move.sd") ||
          Name == "xop.vpcmov" ||
          (Name.startswith("xop.vpcom") && F->arg_size() == 2))) {
       NewFn = nullptr;
       return true;
     }
     // SSE4.1 ptest functions may have an old signature.
     if (IsX86 && Name.startswith("sse41.ptest")) {
       if (Name.substr(11) == "c")
if (NumElts < 8) {
     Cmp = Builder.CreateShuffleVector(Cmp,
                                       Constant::getNullValue(Cmp->getType()),
                                       Indices);
   }
   return Builder.CreateBitCast(Cmp, IntegerType::get(CI.getContext(),
                                                      std::max(NumElts, 8U)));
 }
 
+static Value* upgradeMaskedMove(IRBuilder<> &Builder, CallInst &CI) {
+  Value* A = CI.getArgOperand(0);
+  Value* B = CI.getArgOperand(1);
+  Value* Src = CI.getArgOperand(2);
+  Value* Mask = CI.getArgOperand(3);
+
+  Value* AndNode = Builder.CreateAnd(Mask, APInt(8, 1));
+  Value* Cmp = Builder.CreateIsNotNull(AndNode);
+  Value* Extract1 = Builder.CreateExtractElement(B, (uint64_t)0);
+  Value* Extract2 = Builder.CreateExtractElement(Src, (uint64_t)0);
+  Value* Select = Builder.CreateSelect(Cmp, Extract1, Extract2);
+  return Builder.CreateInsertElement(A, Select, (uint64_t)0);
+}
+
 /// Upgrade a call to an old intrinsic. All argument and return casting must be
 /// provided to seamlessly integrate with existing context.
 void llvm::UpgradeIntrinsicCall(CallInst *CI, Function *NewFn) {
   Function *F = CI->getCalledFunction();
   LLVMContext &C = CI->getContext();
   IRBuilder<> Builder(C);
   Builder.SetInsertPoint(CI->getParent(), CI->getIterator());
 
if (IsX86 && (Name.startswith("sse2.pcmpeq.") ||
                           CI->getArgOperand(2));
     } else if (IsX86 && (Name.startswith("avx512.mask.sub.pd.128") ||
                          Name.startswith("avx512.mask.sub.pd.256") ||
                          Name.startswith("avx512.mask.sub.ps.128") ||
                          Name.startswith("avx512.mask.sub.ps.256"))) {
       Rep = Builder.CreateFSub(CI->getArgOperand(0), CI->getArgOperand(1));
       Rep = EmitX86Select(Builder, CI->getArgOperand(3), Rep,
                           CI->getArgOperand(2));
+    } else if (IsX86 && (Name.startswith("avx512.mask.move.ss") ||
+                         Name.startswith("avx512.mask.move.sd"))) { 
+      Rep = upgradeMaskedMove(Builder, *CI);
     } else {
       llvm_unreachable("Unknown function for CallInst upgrade.");
     }
 
     if (Rep)
       CI->replaceAllUsesWith(Rep);
     CI->eraseFromParent();
     return;

lib/Target/X86/X86IntrinsicsInfo.h

static const IntrinsicData  IntrinsicsWithoutChain[] = {
   X86_INTRINSIC_DATA(avx512_mask_min_ps_128, INTR_TYPE_2OP_MASK, X86ISD::FMIN, 0),
   X86_INTRINSIC_DATA(avx512_mask_min_ps_256, INTR_TYPE_2OP_MASK, X86ISD::FMIN, 0),
   X86_INTRINSIC_DATA(avx512_mask_min_ps_512, INTR_TYPE_2OP_MASK, X86ISD::FMIN,
                      X86ISD::FMIN_RND),
   X86_INTRINSIC_DATA(avx512_mask_min_sd_round, INTR_TYPE_SCALAR_MASK_RM,
                      X86ISD::FMIN_RND, 0),
   X86_INTRINSIC_DATA(avx512_mask_min_ss_round, INTR_TYPE_SCALAR_MASK_RM,
                      X86ISD::FMIN_RND, 0),
-  X86_INTRINSIC_DATA(avx512_mask_move_sd, INTR_TYPE_SCALAR_MASK,
-                     X86ISD::MOVSD, 0),
-  X86_INTRINSIC_DATA(avx512_mask_move_ss, INTR_TYPE_SCALAR_MASK,
-                     X86ISD::MOVSS, 0),
   X86_INTRINSIC_DATA(avx512_mask_mul_pd_512, INTR_TYPE_2OP_MASK, ISD::FMUL,
                      X86ISD::FMUL_RND),
   X86_INTRINSIC_DATA(avx512_mask_mul_ps_512, INTR_TYPE_2OP_MASK, ISD::FMUL,
                      X86ISD::FMUL_RND),
   X86_INTRINSIC_DATA(avx512_mask_mul_sd_round, INTR_TYPE_SCALAR_MASK_RM,
                      X86ISD::FMUL_RND, 0),
   X86_INTRINSIC_DATA(avx512_mask_mul_ss_round, INTR_TYPE_SCALAR_MASK_RM,
                      X86ISD::FMUL_RND, 0),

test/CodeGen/X86/avx512-intrinsics-upgrade.ll

 ; CHECK-NEXT:    vpaddq %zmm3, %zmm2, %zmm0
 ; CHECK-NEXT:    retq
   %res = call <8 x i64> @llvm.x86.avx512.mask.pminu.q.512(<8 x i64> %x0, <8 x i64> %x1, <8 x i64> %x2, i8 %x3)
   %res1 = call <8 x i64> @llvm.x86.avx512.mask.pminu.q.512(<8 x i64> %x0, <8 x i64> %x1, <8 x i64> %x2, i8 -1)
   %res2 = add <8 x i64> %res, %res1
   ret <8 x i64> %res2
 }
 
+define <4 x float> @test_mm_mask_move_ss(<4 x float> %__W, i8 zeroext %__U, <4 x float> %__A, <4 x float> %__B) {
+; CHECK-LABEL: test_mm_mask_move_ss:
+; CHECK:       ## BB#0: ## %entry
+; CHECK-NEXT:    kmovw %edi, %k1
+; CHECK-NEXT:    vmovss %xmm2, %xmm1, %xmm0 {%k1}
+; CHECK-NEXT:    retq
+entry:
+  %res = call <4 x float> @llvm.x86.avx512.mask.move.ss(<4 x float> %__A, <4 x float> %__B, <4 x float> %__W, i8 %__U)
+  ret <4 x float> %res
+}
+
+
+define <4 x float> @test_mm_maskz_move_ss(i8 zeroext %__U, <4 x float> %__A, <4 x float> %__B) {
+; CHECK-LABEL: test_mm_maskz_move_ss:
+; CHECK:       ## BB#0: ## %entry
+; CHECK-NEXT:    kmovw %edi, %k1
+; CHECK-NEXT:    vmovss %xmm1, %xmm0, %xmm0 {%k1} {z}
+; CHECK-NEXT:    retq
+entry:
+  %res = call <4 x float> @llvm.x86.avx512.mask.move.ss(<4 x float> %__A, <4 x float> %__B, <4 x float> zeroinitializer, i8 %__U)
+  ret <4 x float> %res
+}
+
+define <2 x double> @test_mm_mask_move_sd(<2 x double> %__W, i8 zeroext %__U, <2 x double> %__A, <2 x double> %__B) {
+; CHECK-LABEL: test_mm_mask_move_sd:
+; CHECK:       ## BB#0: ## %entry
+; CHECK-NEXT:    kmovw %edi, %k1
+; CHECK-NEXT:    vmovsd %xmm2, %xmm1, %xmm0 {%k1}
+; CHECK-NEXT:    retq
+entry:
+  %res = call <2 x double> @llvm.x86.avx512.mask.move.sd(<2 x double> %__A, <2 x double> %__B, <2 x double> %__W, i8 %__U)
+  ret <2 x double> %res
+}
+
+define <2 x double> @test_mm_maskz_move_sd(i8 zeroext %__U, <2 x double> %__A, <2 x double> %__B) {
+; CHECK-LABEL: test_mm_maskz_move_sd:
+; CHECK:       ## BB#0: ## %entry
+; CHECK-NEXT:    kmovw %edi, %k1
+; CHECK-NEXT:    vmovsd %xmm1, %xmm0, %xmm0 {%k1} {z}
+; CHECK-NEXT:    retq
+entry:
+  %res = call <2 x double> @llvm.x86.avx512.mask.move.sd(<2 x double> %__A, <2 x double> %__B, <2 x double> zeroinitializer, i8 %__U)
+  ret <2 x double> %res
+}
+
+declare <4 x float> @llvm.x86.avx512.mask.move.ss(<4 x float>, <4 x float>, <4 x float>, i8)
+
+declare <2 x double> @llvm.x86.avx512.mask.move.sd(<2 x double>, <2 x double>, <2 x double>, i8)