[X86][FastISel] Use a COPY from K register to a GPR instead of a K operation

[guyblank]

Summary

The KORTEST was introduced due to a bug where a TEST instruction used a K register.
but, turns out that the opposite case of KORTEST using a GPR is now happening (as seen in several bug reports, for example)

The change removes the KORTEST flow and adds a COPY instruction from the K reg to a GPR.

[ABataev]

General comment:
'tmp' is a bad name for a variable, give a meaningful name and start from capital letter (see http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)

[zvi]

Please add a test case for the problem this commit addresses.

[guyblank]

Thanks Alexey & Zvi

update according to your comments.
without the patch, the added test produces a KORTEST with a GPR instead of a TEST.

[zvi]

LGTM

lib/Target/X86/X86FastISel.cpp

bool X86FastISel::X86SelectBranch(const Instruction *I) {
   }
 
   // Otherwise do a clumsy setcc and re-test it.
   // Note that i1 essentially gets ANY_EXTEND'ed to i8 where it isn't used
   // in an explicit cast, so make sure to handle that correctly.
   unsigned OpReg = getRegForValue(BI->getCondition());
   if (OpReg == 0) return false;
 
-  // In case OpReg is a K register, kortest against itself.
-  if (MRI.getRegClass(OpReg) == &X86::VK1RegClass)
-    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::KORTESTWrr))
-        .addReg(OpReg)
-        .addReg(OpReg);
-  else
-    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::TEST8ri))
-        .addReg(OpReg)
-        .addImm(1);
+  // In case OpReg is a K register, COPY to a GPR
+  if (MRI.getRegClass(OpReg) == &X86::VK1RegClass) {
+    unsigned KOpReg = OpReg;
+    OpReg = createResultReg(&X86::GR8RegClass);
+    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+            TII.get(TargetOpcode::COPY), OpReg)
+        .addReg(KOpReg);
+  }
   BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::JNE_1))
     .addMBB(TrueMBB);
   finishCondBranch(BI->getParent(), TrueMBB, FalseMBB);
   return true;
 }
 
 bool X86FastISel::X86SelectShift(const Instruction *I) {
   unsigned CReg = 0, OpReg = 0;
if (NeedTest) {
     // accurate. If we read more than the lsb, we may see non-zero values
     // whereas lsb is zero. Therefore, we have to truncate Op0Reg to i1 for
     // the select. This is achieved by performing TEST against 1.
     unsigned CondReg = getRegForValue(Cond);
     if (CondReg == 0)
       return false;
     bool CondIsKill = hasTrivialKill(Cond);
 
-    // In case OpReg is a K register, kortest against itself.
-    if (MRI.getRegClass(CondReg) == &X86::VK1RegClass)
+    // In case OpReg is a K register, COPY to a GPR
+    if (MRI.getRegClass(CondReg) == &X86::VK1RegClass) {
+      unsigned KCondReg = CondReg;
+      CondReg = createResultReg(&X86::GR8RegClass);
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
-              TII.get(X86::KORTESTWrr))
-          .addReg(CondReg, getKillRegState(CondIsKill))
-          .addReg(CondReg);
-    else
-      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::TEST8ri))
-          .addReg(CondReg, getKillRegState(CondIsKill))
-          .addImm(1);
+              TII.get(TargetOpcode::COPY), CondReg)
+          .addReg(KCondReg, getKillRegState(CondIsKill));
+    }
   }
 
   const Value *LHS = I->getOperand(1);
   const Value *RHS = I->getOperand(2);
 
   unsigned RHSReg = getRegForValue(RHS);
   bool RHSIsKill = hasTrivialKill(RHS);
 
if (CI && (CI->getParent() == I->getParent())) {
     if (!X86FastEmitCompare(CmpLHS, CmpRHS, CmpVT, CI->getDebugLoc()))
       return false;
   } else {
     unsigned CondReg = getRegForValue(Cond);
     if (CondReg == 0)
       return false;
     bool CondIsKill = hasTrivialKill(Cond);
 
-    // In case OpReg is a K register, kortest against itself.
-    if (MRI.getRegClass(CondReg) == &X86::VK1RegClass)
+    // In case OpReg is a K register, COPY to a GPR
+    if (MRI.getRegClass(CondReg) == &X86::VK1RegClass) {
+      unsigned KCondReg = CondReg;
+      CondReg = createResultReg(&X86::GR8RegClass);
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
-              TII.get(X86::KORTESTWrr))
-          .addReg(CondReg, getKillRegState(CondIsKill))
-          .addReg(CondReg);
-    else
-      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::TEST8ri))
-          .addReg(CondReg, getKillRegState(CondIsKill))
-          .addImm(1);
+              TII.get(TargetOpcode::COPY), CondReg)
+          .addReg(KCondReg, getKillRegState(CondIsKill));
+    }
   }
 
   const Value *LHS = I->getOperand(1);
   const Value *RHS = I->getOperand(2);
 
   unsigned LHSReg = getRegForValue(LHS);
   bool LHSIsKill = hasTrivialKill(LHS);
 

test/CodeGen/X86/avx512-fsel.ll

 ; CHECK-NEXT:    movb %dl, %r8b
 ; CHECK-NEXT:    andl $1, %r8d
 ; CHECK-NEXT:    kmovw %r8d, %k0
 ; CHECK-NEXT:    orb %sil, %dil
 ; CHECK-NEXT:    ## implicit-def: %R8D
 ; CHECK-NEXT:    movb %dil, %r8b
 ; CHECK-NEXT:    andl $1, %r8d
 ; CHECK-NEXT:    kmovw %r8d, %k1
-; CHECK-NEXT:    kortestw %k1, %k1
+; CHECK-NEXT:    kmovw %k1, %ecx
+; CHECK-NEXT:    testb $1, %cl
 ; CHECK-NEXT:    movb %al, {{[0-9]+}}(%rsp) ## 1-byte Spill
 ; CHECK-NEXT:    kmovw %k0, {{[0-9]+}}(%rsp) ## 2-byte Spill
 ; CHECK-NEXT:    jne LBB0_1
 ; CHECK-NEXT:    jmp LBB0_2
 ; CHECK-NEXT:  LBB0_1: ## %L_0
 ; CHECK-NEXT:    callq ___assert_rtn
 ; CHECK-NEXT:  LBB0_2: ## %L_1
 ; CHECK-NEXT:    xorl %eax, %eax

test/CodeGen/X86/fast-isel-load-i1.ll

+; RUN: llc < %s -fast-isel -mtriple=x86_64-unknown-linux-gnu -mattr=+avx512f  | FileCheck %s
+
+define i1 @test_i1(i1* %b) {
+; CHECK-LABEL: test_i1:
+; CHECK:       # BB#0: # %entry
+; CHECK-NEXT:    testb $1, (%rdi)
+entry:
+  %0 = load i1, i1* %b, align 1
+  br i1 %0, label %in, label %out
+in:
+  ret i1 0
+out:
+  ret i1 1
+}
+

test/CodeGen/X86/fast-isel-select-cmov.ll

 ; NOAVX512-NEXT:    testb $1, %dil
 ; NOAVX512-NEXT:    cmovew %dx, %si
 ; NOAVX512-NEXT:    movzwl %si, %eax
 ; NOAVX512-NEXT:    retq
 ;
 ; AVX512-LABEL: select_cmov_i16:
 ; AVX512:       ## BB#0:
 ; AVX512-NEXT:    kmovw %edi, %k0
-; AVX512-NEXT:    kortestw %k0, %k0
+; AVX512-NEXT:    kmovw %k0, %eax
+; AVX512-NEXT:    testb $1, %al
 ; AVX512-NEXT:    cmovew %dx, %si
 ; AVX512-NEXT:    movzwl %si, %eax
 ; AVX512-NEXT:    retq
   %1 = select i1 %cond, i16 %a, i16 %b
   ret i16 %1
 }
 
 define zeroext i16 @select_cmp_cmov_i16(i16 zeroext %a, i16 zeroext %b) {

 ; NOAVX512-NEXT:    testb $1, %dil
 ; NOAVX512-NEXT:    cmovel %edx, %esi
 ; NOAVX512-NEXT:    movl %esi, %eax
 ; NOAVX512-NEXT:    retq
 ;
 ; AVX512-LABEL: select_cmov_i32:
 ; AVX512:       ## BB#0:
 ; AVX512-NEXT:    kmovw %edi, %k0
-; AVX512-NEXT:    kortestw %k0, %k0
+; AVX512-NEXT:    kmovw %k0, %eax
+; AVX512-NEXT:    testb $1, %al
 ; AVX512-NEXT:    cmovel %edx, %esi
 ; AVX512-NEXT:    movl %esi, %eax
 ; AVX512-NEXT:    retq
   %1 = select i1 %cond, i32 %a, i32 %b
   ret i32 %1
 }
 
 define i32 @select_cmp_cmov_i32(i32 %a, i32 %b) {

 ; NOAVX512-NEXT:    testb $1, %dil
 ; NOAVX512-NEXT:    cmoveq %rdx, %rsi
 ; NOAVX512-NEXT:    movq %rsi, %rax
 ; NOAVX512-NEXT:    retq
 ;
 ; AVX512-LABEL: select_cmov_i64:
 ; AVX512:       ## BB#0:
 ; AVX512-NEXT:    kmovw %edi, %k0
-; AVX512-NEXT:    kortestw %k0, %k0
+; AVX512-NEXT:    kmovw %k0, %eax
+; AVX512-NEXT:    testb $1, %al
 ; AVX512-NEXT:    cmoveq %rdx, %rsi
 ; AVX512-NEXT:    movq %rsi, %rax
 ; AVX512-NEXT:    retq
   %1 = select i1 %cond, i64 %a, i64 %b
   ret i64 %1
 }
 
 define i64 @select_cmp_cmov_i64(i64 %a, i64 %b) {