[AVX512][FastISel] Do not use K registers in TEST instructions

[guyblank]

Summary

since i1 is legal in AVX512, i1 values weren't promoted to i8, instead K registers were allocated.
this causes an issue when they are passed to a TEST instruction, which is illegal.

Solves Bug 28661

while this patch solves this specific issue, what I would really want to do is make the isel choose GPRs for these i1 values.

[delena]

Any test?

[delena]

lib/Target/X86/X86FastISel.cpp

1694

It may give incorrect result. If the OpReg is k-reg you can use k-shitft-left 15 with ktestw.

[guyblank]

as Elena suggested, changed the sequence to kshift left & ktest.
added a test.

[igorb]

lib/Target/X86/X86FastISel.cpp

1705

KTEST instruction require AVX512DQ, it will fail to run (illegal instruction) on knl target.
We assume that VK1 is always zero extended, you don't need shift.
I suggest to use KORTESTW instruction ( it require only AVX512F , available if VK1 legal ), in this case it have the same semantic.

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

1 ↗

(On Diff #68013)

could you please change it to
; RUN: llc < %s -fast-isel -fast-isel-abort=1 -mtriple=x86_64-apple-darwin10 | FileCheck %s --check-prefix=CHECK --check-prefix=NOAVX512
; RUN: llc < %s -fast-isel -fast-isel-abort=1 -mtriple=x86_64-apple-darwin10 -mattr=+avx512f | FileCheck %s --check-prefix=CHECK --check-prefix=AVX512

and regenerate the checks using update_llc_test_checks utils.

[guyblank]

changes according to igor's comments

[igorb]

lib/Target/X86/X86FastISel.cpp

2052

could you please check if this one should be addReg(CondReg, getKillRegState(CondIsKill)) ?

2236

could you please check if this one should be addReg(CondReg, getKillRegState(CondIsKill))?

[guyblank]

add kill state

[igorb]

LGTM

lib/Target/X86/X86FastISel.cpp

if (TI->hasOneUse() && TI->getParent() == I->getParent() &&
       case MVT::i8:  TestOpc = X86::TEST8ri; break;
       case MVT::i16: TestOpc = X86::TEST16ri; break;
       case MVT::i32: TestOpc = X86::TEST32ri; break;
       case MVT::i64: TestOpc = X86::TEST64ri32; break;
       }
       if (TestOpc) {
         unsigned OpReg = getRegForValue(TI->getOperand(0));
         if (OpReg == 0) return false;
+
         BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(TestOpc))
           .addReg(OpReg).addImm(1);
 
         unsigned JmpOpc = X86::JNE_1;
         if (FuncInfo.MBB->isLayoutSuccessor(TrueMBB)) {
           std::swap(TrueMBB, FalseMBB);
           JmpOpc = X86::JE_1;
         }
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, move it to a GPR to be able to use the
+  // TEST instruction.
+  if (MRI.getRegClass(OpReg) == &X86::VK1RegClass) {
+    unsigned tmp = OpReg;
+    OpReg = createResultReg(&X86::GR8RegClass);
+    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+            TII.get(TargetOpcode::COPY), OpReg)
+        .addReg(tmp);
+  }
   BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::TEST8ri))
     .addReg(OpReg).addImm(1);
   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;
   const TargetRegisterClass *RC = nullptr;
   if (I->getType()->isIntegerTy(8)) {
     CReg = X86::CL;
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 CondReg is a K register, move it to a GPR to be able to use the
+    // TEST instruction.
+    if (MRI.getRegClass(CondReg) == &X86::VK1RegClass) {
+      unsigned tmp = CondReg;
+      CondReg = createResultReg(&X86::GR8RegClass);
+      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+              TII.get(TargetOpcode::COPY), CondReg)
+          .addReg(tmp, getKillRegState(CondIsKill));
+    }
+
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::TEST8ri))
       .addReg(CondReg, getKillRegState(CondIsKill)).addImm(1);
   }
 
   const Value *LHS = I->getOperand(1);
   const Value *RHS = I->getOperand(2);
 
   unsigned RHSReg = getRegForValue(RHS);
   bool RHSIsKill = hasTrivialKill(RHS);
 
   unsigned LHSReg = getRegForValue(LHS);
   bool LHSIsKill = hasTrivialKill(LHS);
 
   if (!LHSReg || !RHSReg)
     return false;
if (CI && (CI->getParent() == I->getParent())) {
     EVT CmpVT = TLI.getValueType(DL, CmpLHS->getType());
     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 CondReg is a K register, move it to a GPR to be able to use the
+    // TEST instruction.
+    if (MRI.getRegClass(CondReg) == &X86::VK1RegClass) {
+      unsigned tmp = CondReg;
+      CondReg = createResultReg(&X86::GR8RegClass);
+      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+              TII.get(TargetOpcode::COPY), CondReg)
+          .addReg(tmp, getKillRegState(CondIsKill));
+    }
+
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(X86::TEST8ri))
       .addReg(CondReg, getKillRegState(CondIsKill)).addImm(1);
   }
 
   const Value *LHS = I->getOperand(1);
   const Value *RHS = I->getOperand(2);
 
   unsigned LHSReg = getRegForValue(LHS);
   bool LHSIsKill = hasTrivialKill(LHS);
 
   unsigned RHSReg = getRegForValue(RHS);
   bool RHSIsKill = hasTrivialKill(RHS);
 
   if (!LHSReg || !RHSReg)
     return false;