Merge compares and branches

src/IceTargetLoweringX86BaseImpl.h

 //===- subzero/src/IceTargetLoweringX86BaseImpl.h - x86 lowering -*- C++ -*-==//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 105 matching lines @@
 template <class MachineTraits>
 BoolFoldingEntry<MachineTraits>::BoolFoldingEntry(Inst *I)
     : Instr(I), IsComplex(BoolFolding<MachineTraits>::hasComplexLowering(I)) {}
 
 template <class MachineTraits>
 typename BoolFolding<MachineTraits>::BoolFoldingProducerKind
 BoolFolding<MachineTraits>::getProducerKind(const Inst *Instr) {
   if (llvm::isa<InstIcmp>(Instr)) {
     if (MachineTraits::Is64Bit || Instr->getSrc(0)->getType() != IceType_i64)
       return PK_Icmp32;
-    return PK_None; // TODO(stichnot): actually PK_Icmp64;
+    return PK_Icmp64;
   }
   return PK_None; // TODO(stichnot): remove this
 
   if (llvm::isa<InstFcmp>(Instr))
     return PK_Fcmp;
   if (auto *Cast = llvm::dyn_cast<InstCast>(Instr)) {
     switch (Cast->getCastKind()) {
     default:
       return PK_None;
     case InstCast::Trunc:
@@ skipping 56 matching lines @@
       Producers[Var->getIndex()] = BoolFoldingEntry<MachineTraits>(&Instr);
     }
     // Check each src variable against the map.
     FOREACH_VAR_IN_INST(Var, Instr) {
       SizeT VarNum = Var->getIndex();
       if (containsValid(VarNum)) {
         if (IndexOfVarOperandInInst(Var) !=
                 0 // All valid consumers use Var as the first source operand
             ||
             getConsumerKind(&Instr) == CK_None // must be white-listed
+            ||
+            (getConsumerKind(&Instr) != CK_Br && // Icmp64 only folds in branches

[Jim Stichnoth, 2015/10/16 13:58:11]

80-col

[sehr, 2015/10/16 18:10:22]

Done.

+             getProducerKind(Producers[VarNum].Instr) != PK_Icmp32)
             || (Producers[VarNum].IsComplex && // complex can't be multi-use
                 Producers[VarNum].NumUses > 0)) {
           setInvalid(VarNum);
           continue;
         }
         ++Producers[VarNum].NumUses;
         if (Instr.isLastUse(Var)) {
           Producers[VarNum].IsLiveOut = false;
         }
       }
@@ skipping 1695 matching lines @@
     return;
   }
   Operand *Cond = Inst->getCondition();
 
   // Handle folding opportunities.
   if (const class Inst *Producer = FoldingInfo.getProducerFor(Cond)) {
     assert(Producer->isDeleted());
     switch (BoolFolding::getProducerKind(Producer)) {
     default:
       break;
-    case BoolFolding::PK_Icmp32: {
-      // TODO(stichnot): Refactor similarities between this block and the
-      // corresponding code in lowerIcmp().
-      auto *Cmp = llvm::dyn_cast<InstIcmp>(Producer);
-      Operand *Src0 = Producer->getSrc(0);
-      Operand *Src1 = legalize(Producer->getSrc(1));
-      Operand *Src0RM = legalizeSrc0ForCmp(Src0, Src1);
-      _cmp(Src0RM, Src1);
-      _br(Traits::getIcmp32Mapping(Cmp->getCondition()), Inst->getTargetTrue(),
-          Inst->getTargetFalse());
+    case BoolFolding::PK_Icmp32:
+    case BoolFolding::PK_Icmp64: {
+      lowerIcmpAndBr(llvm::dyn_cast<InstIcmp>(Producer), Inst);
       return;
     }
     }
   }
 
   Operand *Src0 = legalize(Cond, Legal_Reg | Legal_Mem);
   Constant *Zero = Ctx->getConstantZero(IceType_i32);
   _cmp(Src0, Zero);
   _br(Traits::Cond::Br_ne, Inst->getTargetTrue(), Inst->getTargetFalse());
 }
@@ skipping 713 matching lines @@
 inline bool isZero(const Operand *Opnd) {
   if (auto *C64 = llvm::dyn_cast<ConstantInteger64>(Opnd))
     return C64->getValue() == 0;
   if (auto *C32 = llvm::dyn_cast<ConstantInteger32>(Opnd))
     return C32->getValue() == 0;
   return false;
 }
 
 template <class Machine>
 void TargetX86Base<Machine>::lowerIcmp(const InstIcmp *Inst) {
-  Operand *Src0 = legalize(Inst->getSrc(0));
-  Operand *Src1 = legalize(Inst->getSrc(1));
-  Variable *Dest = Inst->getDest();
+  lowerIcmpAndBr(Inst, nullptr);

[Jim Stichnoth, 2015/10/16 13:58:11]

Maybe use "constexpr InstBr *Br = nullptr;" for clarity?

[sehr, 2015/10/16 18:10:22]

Done.

+}
+
+template <class Machine>
+void TargetX86Base<Machine>::lowerIcmpAndBr(const InstIcmp *Icmp,
+                                            const InstBr *Br) {
+  Operand *Src0 = legalize(Icmp->getSrc(0));
+  Operand *Src1 = legalize(Icmp->getSrc(1));
+  Variable *Dest = Icmp->getDest();
 
   if (isVectorType(Dest->getType())) {
+    assert((Br == nullptr) && "not ready to fold vector compare/branch");

[Jim Stichnoth, 2015/10/16 13:58:11]

Make this a report_fatal_error() instead?

Also, I can't see that we would *ever* be doing branch optimizations on bool
vector operations, so rephrase the error not to get people's hopes up? :)

[sehr, 2015/10/16 18:10:22]

Done.

     Type Ty = Src0->getType();
     // Promote i1 vectors to 128 bit integer vector types.
     if (typeElementType(Ty) == IceType_i1) {
       Type NewTy = IceType_NUM;
       switch (Ty) {
       default:
         llvm_unreachable("unexpected type");
         break;
       case IceType_v4i1:
         NewTy = IceType_v4i32;
         break;
       case IceType_v8i1:
         NewTy = IceType_v8i16;
         break;
       case IceType_v16i1:
         NewTy = IceType_v16i8;
         break;
       }
       Variable *NewSrc0 = Func->makeVariable(NewTy);
       Variable *NewSrc1 = Func->makeVariable(NewTy);
       lowerCast(InstCast::create(Func, InstCast::Sext, NewSrc0, Src0));
       lowerCast(InstCast::create(Func, InstCast::Sext, NewSrc1, Src1));
       Src0 = NewSrc0;
       Src1 = NewSrc1;
       Ty = NewTy;
     }
 
-    InstIcmp::ICond Condition = Inst->getCondition();
+    InstIcmp::ICond Condition = Icmp->getCondition();
 
     Operand *Src0RM = legalize(Src0, Legal_Reg | Legal_Mem);
     Operand *Src1RM = legalize(Src1, Legal_Reg | Legal_Mem);
 
     // SSE2 only has signed comparison operations. Transform unsigned inputs in
     // a manner that allows for the use of signed comparison operations by
     // flipping the high order bits.
     if (Condition == InstIcmp::Ugt || Condition == InstIcmp::Uge ||
         Condition == InstIcmp::Ult || Condition == InstIcmp::Ule) {
       Variable *T0 = makeReg(Ty);
@@ skipping 61 matching lines @@
       _pxor(T, MinusOne);
     } break;
     }
 
     _movp(Dest, T);
     eliminateNextVectorSextInstruction(Dest);
     return;
   }
 
   if (!Traits::Is64Bit && Src0->getType() == IceType_i64) {
-    lowerIcmp64(Inst);
+    lowerIcmp64(Icmp, Br);
     return;
   }
 
   // cmp b, c
   if (isZero(Src1)) {
-    switch (Inst->getCondition()) {
+    switch (Icmp->getCondition()) {
     default:
       break;
     case InstIcmp::Uge:
-      _mov(Dest, Ctx->getConstantInt(Dest->getType(), 1));
+      movOrBr(true, Dest, Br);
       return;
     case InstIcmp::Ult:
-      _mov(Dest, Ctx->getConstantInt(Dest->getType(), 0));
+      movOrBr(false, Dest, Br);
       return;
     }
   }
   Operand *Src0RM = legalizeSrc0ForCmp(Src0, Src1);
   _cmp(Src0RM, Src1);
-  _setcc(Dest, Traits::getIcmp32Mapping(Inst->getCondition()));
+  setccOrBr(Traits::getIcmp32Mapping(Icmp->getCondition()), Dest, Br);
 }
 
 template <typename Machine>
 template <typename T>
 typename std::enable_if<!T::Is64Bit, void>::type
-TargetX86Base<Machine>::lowerIcmp64(const InstIcmp *Inst) {
+TargetX86Base<Machine>::lowerIcmp64(const InstIcmp *Icmp, const InstBr *Br) {
   // a=icmp cond, b, c ==> cmp b,c; a=1; br cond,L1; FakeUse(a); a=0; L1:
-  Operand *Src0 = legalize(Inst->getSrc(0));
-  Operand *Src1 = legalize(Inst->getSrc(1));
-  Variable *Dest = Inst->getDest();
-  InstIcmp::ICond Condition = Inst->getCondition();
+  Operand *Src0 = legalize(Icmp->getSrc(0));
+  Operand *Src1 = legalize(Icmp->getSrc(1));
+  Variable *Dest = Icmp->getDest();
+  InstIcmp::ICond Condition = Icmp->getCondition();
   size_t Index = static_cast<size_t>(Condition);
   assert(Index < Traits::TableIcmp64Size);
   Constant *Zero = Ctx->getConstantZero(IceType_i32);
   Constant *One = Ctx->getConstantInt32(1);
   Operand *Src0LoRM = nullptr;
   Operand *Src0HiRM = nullptr;
   // Legalize the portions of Src0 that are going to be needed.
   if (isZero(Src1)) {
     switch (Condition) {
     default:
@@ skipping 32 matching lines @@
     Variable *Temp = nullptr;
     switch (Condition) {
     default:
       llvm_unreachable("unexpected condition");
       break;
     case InstIcmp::Eq:
     case InstIcmp::Ule:
       _mov(Temp, Src0LoRM);
       _or(Temp, Src0HiRM);
       Context.insert(InstFakeUse::create(Func, Temp));
-      _setcc(Dest, Traits::Cond::Br_e);
+      setccOrBr(Traits::Cond::Br_e, Dest, Br);
       return;
     case InstIcmp::Ne:
     case InstIcmp::Ugt:
       _mov(Temp, Src0LoRM);
       _or(Temp, Src0HiRM);
       Context.insert(InstFakeUse::create(Func, Temp));
-      _setcc(Dest, Traits::Cond::Br_ne);
+      setccOrBr(Traits::Cond::Br_ne, Dest, Br);
       return;
     case InstIcmp::Uge:
-      _mov(Dest, Ctx->getConstantInt(Dest->getType(), 1));
+      movOrBr(true, Dest, Br);
       return;
     case InstIcmp::Ult:
-      _mov(Dest, Ctx->getConstantInt(Dest->getType(), 0));
+      movOrBr(false, Dest, Br);
       return;
     case InstIcmp::Sgt:
       break;
     case InstIcmp::Sge:
       _test(Src0HiRM, SignMask);
-      _setcc(Dest, Traits::Cond::Br_e);
+      setccOrBr(Traits::Cond::Br_e, Dest, Br);
       return;
     case InstIcmp::Slt:
       _test(Src0HiRM, SignMask);
-      _setcc(Dest, Traits::Cond::Br_ne);
+      setccOrBr(Traits::Cond::Br_ne, Dest, Br);
       return;
     case InstIcmp::Sle:
       break;
     }
   }
   // Handle general compares.
   Operand *Src1LoRI = legalize(loOperand(Src1), Legal_Reg | Legal_Imm);
   Operand *Src1HiRI = legalize(hiOperand(Src1), Legal_Reg | Legal_Imm);
-  typename Traits::Insts::Label *LabelFalse =
-      Traits::Insts::Label::create(Func, this);
-  typename Traits::Insts::Label *LabelTrue =
-      Traits::Insts::Label::create(Func, this);
-  _mov(Dest, One);
-  _cmp(Src0HiRM, Src1HiRI);
-  if (Traits::TableIcmp64[Index].C1 != Traits::Cond::Br_None)
-    _br(Traits::TableIcmp64[Index].C1, LabelTrue);
-  if (Traits::TableIcmp64[Index].C2 != Traits::Cond::Br_None)
-    _br(Traits::TableIcmp64[Index].C2, LabelFalse);
-  _cmp(Src0LoRM, Src1LoRI);
-  _br(Traits::TableIcmp64[Index].C3, LabelTrue);
-  Context.insert(LabelFalse);
-  _mov_redefined(Dest, Zero);
-  Context.insert(LabelTrue);
+  if (Br == nullptr) {
+    typename Traits::Insts::Label *LabelFalse =
+        Traits::Insts::Label::create(Func, this);
+    typename Traits::Insts::Label *LabelTrue =
+        Traits::Insts::Label::create(Func, this);
+    _mov(Dest, One);
+    _cmp(Src0HiRM, Src1HiRI);
+    if (Traits::TableIcmp64[Index].C1 != Traits::Cond::Br_None)
+      _br(Traits::TableIcmp64[Index].C1, LabelTrue);
+    if (Traits::TableIcmp64[Index].C2 != Traits::Cond::Br_None)
+      _br(Traits::TableIcmp64[Index].C2, LabelFalse);
+    _cmp(Src0LoRM, Src1LoRI);
+    _br(Traits::TableIcmp64[Index].C3, LabelTrue);
+    Context.insert(LabelFalse);
+    _mov_redefined(Dest, Zero);
+    Context.insert(LabelTrue);
+  } else {
+    _cmp(Src0HiRM, Src1HiRI);
+    if (Traits::TableIcmp64[Index].C1 != Traits::Cond::Br_None)
+      _br(Traits::TableIcmp64[Index].C1, Br->getTargetTrue());
+    if (Traits::TableIcmp64[Index].C2 != Traits::Cond::Br_None)
+      _br(Traits::TableIcmp64[Index].C2, Br->getTargetFalse());
+    _cmp(Src0LoRM, Src1LoRI);
+    _br(Traits::TableIcmp64[Index].C3, Br->getTargetTrue(), Br->getTargetFalse());

[Jim Stichnoth, 2015/10/16 13:58:11]

80-col

[sehr, 2015/10/16 18:10:22]

Done.

+  }
 }
 
 template <class Machine>
+void TargetX86Base<Machine>::setccOrBr(typename Traits::Cond::BrCond Condition,
+                                       Variable *Dest, const InstBr *Br) {
+  if (Br == nullptr) {
+    _setcc(Dest, Condition);
+  } else {
+    _br(Condition, Br->getTargetTrue(), Br->getTargetFalse());
+  }
+}
+
+template <class Machine>
+void TargetX86Base<Machine>::movOrBr(bool IcmpResult, Variable *Dest,
+                                     const InstBr *Br) {
+  if (Br == nullptr) {
+    _mov(Dest, Ctx->getConstantInt(Dest->getType(), (IcmpResult ? 1 : 0)));
+  } else {
+    // TODO(sehr,stichnot): This could be done with a single unconditional
+    // branch instruction, but subzero doesn't know how to handle the resulting
+    // control flow graph changes now.  Make it do so to eliminate mov and cmp.
+    _mov(Dest, Ctx->getConstantInt(Dest->getType(), (IcmpResult ? 1 : 0)));
+    _cmp(Dest, Ctx->getConstantInt(Dest->getType(), 0));
+    _br(Traits::Cond::Br_ne, Br->getTargetTrue(), Br->getTargetFalse());
+  }
+}
+
+template <class Machine>
 void TargetX86Base<Machine>::lowerInsertElement(const InstInsertElement *Inst) {
   Operand *SourceVectNotLegalized = Inst->getSrc(0);
   Operand *ElementToInsertNotLegalized = Inst->getSrc(1);
   ConstantInteger32 *ElementIndex =
       llvm::dyn_cast<ConstantInteger32>(Inst->getSrc(2));
   // Only constant indices are allowed in PNaCl IR.
   assert(ElementIndex);
   unsigned Index = ElementIndex->getValue();
   assert(Index < typeNumElements(SourceVectNotLegalized->getType()));
 
@@ skipping 2687 matching lines @@
   }
   // the offset is not eligible for blinding or pooling, return the original
   // mem operand
   return MemOperand;
 }
 
 } // end of namespace X86Internal
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICETARGETLOWERINGX86BASEIMPL_H