Optimize 64-bit compares with zero

src/IceTargetLoweringX86BaseImpl.h

Index: src/IceTargetLoweringX86BaseImpl.h
diff --git a/src/IceTargetLoweringX86BaseImpl.h b/src/IceTargetLoweringX86BaseImpl.h
index 0b3a32f87c0b0c283d946e971b4f73fc0ebca0cc..b1ce15e59650e71a5f86a5ca1cd88ebafdf869e7 100644
--- a/src/IceTargetLoweringX86BaseImpl.h
+++ b/src/IceTargetLoweringX86BaseImpl.h
@@ -2769,6 +2769,25 @@ void TargetX86Base<Machine>::lowerIcmp(const InstIcmp *Inst) {
   }
 
   // cmp b, c
+  if (auto *Const = llvm::dyn_cast<ConstantInteger32>(Src1)) {
+    Type Ty = Src0->getType();
+    if ((Const->getValue() == 0) &&
+        (Ty == IceType_i32 || Ty == IceType_i16 || Ty == IceType_i8)) {

[Jim Stichnoth, 2015/10/15 00:48:47]

Do you need these tests on Ty?

Vector and FP types are already covered above.

It seems like it should work just fine for I64, except the dyn_cast won't
trigger on ConstantInteger64.

I think I1 should be fine as well.

[sehr, 2015/10/15 17:33:45]

Removed.

+      Constant *Zero = Ctx->getConstantInt1(0);
+      Constant *One = Ctx->getConstantInt1(1);
+      InstIcmp::ICond Condition = Inst->getCondition();
+      switch (Condition) {
+      default:
+        break;
+      case InstIcmp::Uge:
+        _mov(Dest, One);
+        return;
+      case InstIcmp::Ult:
+        _mov(Dest, Zero);
+        return;
+      }
+    }
+  }
   Operand *Src0RM = legalizeSrc0ForCmp(Src0, Src1);
   _cmp(Src0RM, Src1);
   _setcc(Dest, Traits::getIcmp32Mapping(Inst->getCondition()));
@@ -2785,12 +2804,90 @@ TargetX86Base<Machine>::lowerIcmp64(const InstIcmp *Inst) {
   InstIcmp::ICond Condition = Inst->getCondition();
   size_t Index = static_cast<size_t>(Condition);
   assert(Index < Traits::TableIcmp64Size);
-  Operand *Src0LoRM = legalize(loOperand(Src0), Legal_Reg | Legal_Mem);
-  Operand *Src0HiRM = legalize(hiOperand(Src0), Legal_Reg | Legal_Mem);
-  Operand *Src1LoRI = legalize(loOperand(Src1), Legal_Reg | Legal_Imm);
-  Operand *Src1HiRI = legalize(hiOperand(Src1), Legal_Reg | Legal_Imm);
   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 (llvm::isa<ConstantInteger64>(Src1) &&

[Jim Stichnoth, 2015/10/15 00:48:47]

I don't like isa<> followed by cast<> or dyn_cast<>.

How about a helper in an anonymous namespace:

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;
}

That helper could probably also be used above.

[sehr, 2015/10/15 17:33:45]

Done.

+      (llvm::dyn_cast<ConstantInteger64>(Src1)->getValue() == 0)) {
+    switch (Condition) {
+    default:
+      llvm_unreachable("unexpected condition");
+      break;
+    // These two are not optimized, so we fall through to the general case,
+    // which needs the upper and lower halves legalized.
+    case InstIcmp::Sgt:
+    case InstIcmp::Sle:
+    // These four or the high and low half and compare, so they need the

[Jim Stichnoth, 2015/10/15 00:48:47]

s/or/are/ ?

[sehr, 2015/10/15 17:33:46]

Reworded to avoid the convoluted mis-parse possible here.

+    // upper and lower halves legalized.
+    case InstIcmp::Eq:
+    case InstIcmp::Ule:
+    case InstIcmp::Ne:
+    case InstIcmp::Ugt:
+      Src0LoRM = legalize(loOperand(Src0), Legal_Reg | Legal_Mem);
+    // These two test only the high half's sign bit, so they need only
+    // the upper half legalized.
+    case InstIcmp::Sge:
+    case InstIcmp::Slt:
+      Src0HiRM = legalize(hiOperand(Src0), Legal_Reg | Legal_Mem);
+      break;
+
+    // These two move constants and hence need no legalization.
+    case InstIcmp::Uge:
+    case InstIcmp::Ult:
+      break;
+    }
+  } else {
+    Src0LoRM = legalize(loOperand(Src0), Legal_Reg | Legal_Mem);
+    Src0HiRM = legalize(hiOperand(Src0), Legal_Reg | Legal_Mem);
+  }
+  // Optimize the comparisons with zero.
+  if (llvm::isa<ConstantInteger64>(Src1) &&
+      (llvm::dyn_cast<ConstantInteger64>(Src1)->getValue() == 0)) {
+    Constant *SignMask = Ctx->getConstantInt32(0x80000000);
+    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);
+      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);
+      return;
+    case InstIcmp::Uge:
+      _mov(Dest, Ctx->getConstantInt1(1));
+      return;
+    case InstIcmp::Ult:
+      _mov(Dest, Ctx->getConstantInt1(0));
+      return;
+    case InstIcmp::Sgt:
+      break;
+    case InstIcmp::Sge:
+      _test(Src0HiRM, SignMask);
+      _setcc(Dest, Traits::Cond::Br_e);
+      return;
+    case InstIcmp::Slt:
+      _test(Src0HiRM, SignMask);
+      _setcc(Dest, Traits::Cond::Br_ne);
+      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 =