Chris,
I rewrote the llvm-gcc portion of the SETCC patch with your suggestions.
It is much cleaner now. The patch is attached. Hopefully this is more to
your liking.
Reid.
Sheng: This is just FYI.
On Sun, 2006-12-10 at 18:03 -0800, Chris Lattner wrote:
> On Dec 10, 2006, at 3:43 PM, Reid Spencer wrote:
> > Here's the SETCC patch to convert SetCondInst (SetCC) instructions
> > into
> > ICmpInst. Here's somethings you should know about this patch:
> >
> >
> > 3. The SetCondInst instruction is still used for floating point
> > comparisons.
>
>
> Ok, this will be changed in the next patch?
>
>
> > 6. The llvm-gcc patch emits ICmp for integer/pointer and SetCC for
> > floating
> > point. No FCmpInst instructions are emitted. We'll do that in the
> > next patch.
>
>
> Ok, sounds great.
>
>
> For the llvm-gcc patch:
>
>
> Value *EmitCompare(tree_node *exp, unsigned Opc, bool isUnordered);
> + Value *EmitCompare(tree_node *exp, unsigned Opc);
>
>
> I don't like the subtle overloads here. One way to fix this:
>
>
> + case LT_EXPR: {
> + tree Op0Ty = TREE_TYPE(TREE_OPERAND(exp,0));
> + if (!FLOAT_TYPE_P(Op0Ty))
> + Result =
> + EmitCompare(exp, TYPE_UNSIGNED(Op0Ty) ?
> + ICmpInst::ICMP_ULT : ICmpInst::ICMP_SLT);
> + else
> + Result = EmitCompare(exp, Instruction::SetLT, 0);
> + break;
> + }
> + case LE_EXPR: {
> + tree Op0Ty = TREE_TYPE(TREE_OPERAND(exp,0));
> + if (!FLOAT_TYPE_P(Op0Ty))
> + Result =
> + EmitCompare(exp, TYPE_UNSIGNED(Op0Ty) ?
> + ICmpInst::ICMP_ULE : ICmpInst::ICMP_SLE);
> + else
> + Result = EmitCompare(exp, Instruction::SetLE, 0);
> + break;
> + }
>
>
> This logic shouldn't be duplicated everywhere. I much prefer that you
> do something like:
>
>
> case LT_EXPR:
> Result = EmitCompare(exp, ICmpInst::ICMP_ULT,
> ICmpInst::ICMP_SLT, Instruction::SetLT, 0);
> break;
>
>
> ... and move the code for determining which opcode to use into
> EmitCompare. The same can be
> done for EmitMinMaxExpr.
>
>
>
>
> @@ -2261,8 +2319,8 @@
> Value *Op = Emit(TREE_OPERAND(exp, 0), 0);
> if (!Op->getType()->isFloatingPoint()) {
> Instruction *OpN = BinaryOperator::createNeg(Op,
> Op->getName()+"neg",CurBB);
> - Value *Cmp = new SetCondInst(Instruction::SetGE, Op,
> OpN->getOperand(0),
> - "abscond", CurBB);
> + Value *Cmp = new ICmpInst(ICmpInst::ICMP_SGE, Op,
> OpN->getOperand(0),
> + "abscond", CurBB);
> return new SelectInst(Cmp, Op, OpN, "abs", CurBB);
> } else {
> // Turn FP abs into fabs/fabsf.
>
>
> This isn't right. You need to emit a signed or unsigned comparison
> depending on TYPE_UNSIGNED. It would make sense to just use a call to
> EmitCompare here and have it pick the right one. I know that unsigned
> abs doesn't make much sense, but this is how expr.c handles it for
> GCC:
>
>
> /* Unsigned abs is simply the operand. Testing here means we
> don't
> risk generating incorrect code below. */
> if (TYPE_UNSIGNED (type))
> return op0;
>
>
> @@ -2464,8 +2542,13 @@
> LHS = NOOPCastToType(LHS, Ty);
> RHS = NOOPCastToType(RHS, Ty);
>
>
>
>
> - Value *Pred = new SetCondInst((Instruction::BinaryOps)CmpOpc, LHS,
> RHS,
> - "tmp", CurBB);
> + Value *Pred;
> + if (LHS->getType()->isFloatingPoint())
> + Pred = new SetCondInst((Instruction::BinaryOps)CmpOpc, LHS, RHS,
> + "tmp", CurBB);
> + else
> + Pred = new ICmpInst((ICmpInst::Predicate)CmpOpc, LHS, RHS,
> + "tmp", CurBB);
> return new SelectInst(Pred, LHS, RHS,
> TREE_CODE(exp) == MAX_EXPR ? "max" : "min",
> CurBB);
> }
>
>
> Likewise this code is wrong (for max). It should also just call
> EmitCompare.
>
>
> -Chris
Index: gcc/llvm-convert.cpp
===================================================================
--- gcc/llvm-convert.cpp (revision 225)
+++ gcc/llvm-convert.cpp (working copy)
@@ -537,23 +537,35 @@
case TRUTH_NOT_EXPR: Result = EmitTRUTH_NOT_EXPR(exp); break;
// Binary Operators
- case LT_EXPR: Result = EmitCompare(exp, Instruction::SetLT, 0); break;
- case LE_EXPR: Result = EmitCompare(exp, Instruction::SetLE, 0); break;
- case GT_EXPR: Result = EmitCompare(exp, Instruction::SetGT, 0); break;
- case GE_EXPR: Result = EmitCompare(exp, Instruction::SetGE, 0); break;
- case EQ_EXPR: Result = EmitCompare(exp, Instruction::SetEQ, 0); break;
- case NE_EXPR: Result = EmitCompare(exp, Instruction::SetNE, 0); break;
- case UNORDERED_EXPR: Result = EmitCompare(exp, 0, 1); break; // not a typo
- case ORDERED_EXPR: Result = EmitCompare(exp, 0, 1); break; // not a typo
- case UNLT_EXPR: Result = EmitCompare(exp, Instruction::SetLT, 1); break;
- case UNLE_EXPR: Result = EmitCompare(exp, Instruction::SetLE, 1); break;
- case UNGT_EXPR: Result = EmitCompare(exp, Instruction::SetGT, 1); break;
- case UNGE_EXPR: Result = EmitCompare(exp, Instruction::SetGE, 1); break;
- case UNEQ_EXPR: Result = EmitCompare(exp, Instruction::SetEQ, 1); break;
- case LTGT_EXPR: Result = EmitCompare(exp, Instruction::SetNE, 1); break;
- case PLUS_EXPR: Result = EmitBinOp(exp, DestLoc, Instruction::Add);break;
- case MINUS_EXPR: Result = EmitBinOp(exp, DestLoc, Instruction::Sub);break;
- case MULT_EXPR: Result = EmitBinOp(exp, DestLoc, Instruction::Mul);break;
+ case LT_EXPR:
+ Result = EmitCompare(exp, ICmpInst::ICMP_ULT, ICmpInst::ICMP_SLT,
+ Instruction::SetLT, 0); break;
+ case LE_EXPR:
+ Result = EmitCompare(exp, ICmpInst::ICMP_ULE, ICmpInst::ICMP_SLE,
+ Instruction::SetLE, 0); break;
+ case GT_EXPR:
+ Result = EmitCompare(exp, ICmpInst::ICMP_UGT, ICmpInst::ICMP_SGT,
+ Instruction::SetGT, 0); break;
+ case GE_EXPR:
+ Result = EmitCompare(exp, ICmpInst::ICMP_UGE, ICmpInst::ICMP_SGE,
+ Instruction::SetGE, 0); break;
+ case EQ_EXPR:
+ Result = EmitCompare(exp, ICmpInst::ICMP_EQ, ICmpInst::ICMP_EQ,
+ Instruction::SetEQ, 0); break;
+ case NE_EXPR:
+ Result = EmitCompare(exp, ICmpInst::ICMP_NE, ICmpInst::ICMP_NE,
+ Instruction::SetNE, 0); break;
+ case UNORDERED_EXPR: Result = EmitCompare(exp, 0, 0, 0, 1); break;// not typo
+ case ORDERED_EXPR: Result = EmitCompare(exp, 0, 0, 0, 1); break;// not typo
+ case UNLT_EXPR: Result = EmitCompare(exp, 0, 0, Instruction::SetLT, 1); break;
+ case UNLE_EXPR: Result = EmitCompare(exp, 0, 0, Instruction::SetLE, 1); break;
+ case UNGT_EXPR: Result = EmitCompare(exp, 0, 0, Instruction::SetGT, 1); break;
+ case UNGE_EXPR: Result = EmitCompare(exp, 0, 0, Instruction::SetGE, 1); break;
+ case UNEQ_EXPR: Result = EmitCompare(exp, 0, 0, Instruction::SetEQ, 1); break;
+ case LTGT_EXPR: Result = EmitCompare(exp, 0, 0, Instruction::SetNE, 1); break;
+ case PLUS_EXPR: Result = EmitBinOp(exp, DestLoc, Instruction::Add);break;
+ case MINUS_EXPR:Result = EmitBinOp(exp, DestLoc, Instruction::Sub);break;
+ case MULT_EXPR: Result = EmitBinOp(exp, DestLoc, Instruction::Mul);break;
case TRUNC_DIV_EXPR:
if (TYPE_UNSIGNED(TREE_TYPE(exp)))
Result = EmitBinOp(exp, DestLoc, Instruction::UDiv);
@@ -589,8 +601,14 @@
case LROTATE_EXPR:
Result = EmitRotateOp(exp, Instruction::Shl, Instruction::LShr);
break;
- case MIN_EXPR: Result = EmitMinMaxExpr(exp, Instruction::SetLE); break;
- case MAX_EXPR: Result = EmitMinMaxExpr(exp, Instruction::SetGE); break;
+ case MIN_EXPR:
+ Result = EmitMinMaxExpr(exp, ICmpInst::ICMP_ULE, ICmpInst::ICMP_SLE,
+ Instruction::SetLE);
+ break;
+ case MAX_EXPR:
+ Result = EmitMinMaxExpr(exp, ICmpInst::ICMP_UGE, ICmpInst::ICMP_SGE,
+ Instruction::SetGE);
+ break;
case CONSTRUCTOR: Result = EmitCONSTRUCTOR(exp, DestLoc); break;
// Complex Math Expressions.
@@ -2269,8 +2287,9 @@
Value *Op = Emit(TREE_OPERAND(exp, 0), 0);
if (!Op->getType()->isFloatingPoint()) {
Instruction *OpN = BinaryOperator::createNeg(Op, Op->getName()+"neg",CurBB);
- Value *Cmp = new SetCondInst(Instruction::SetGE, Op, OpN->getOperand(0),
- "abscond", CurBB);
+ ICmpInst::Predicate pred = TYPE_UNSIGNED(TREE_TYPE(TREE_OPERAND(exp,0))) ?
+ ICmpInst::ICMP_UGE : ICmpInst::ICMP_SGE;
+ Value *Cmp = new ICmpInst(pred, Op, OpN->getOperand(0), "abscond", CurBB);
return new SelectInst(Cmp, Op, OpN, "abs", CurBB);
} else {
// Turn FP abs into fabs/fabsf.
@@ -2295,20 +2314,38 @@
/// comparison to use. isUnord is true if this is a floating point comparison
/// that should also be true if either operand is a NaN. Note that Opc can be
/// set to zero for special cases.
-Value *TreeToLLVM::EmitCompare(tree exp, unsigned Opc, bool isUnord) {
- if (TREE_CODE(TREE_TYPE(TREE_OPERAND(exp, 0))) == COMPLEX_TYPE)
- return EmitComplexBinOp(exp, 0); // Complex ==/!=
-
- // Comparison of struct is not allowed, so this is safe.
+Value *TreeToLLVM::EmitCompare(tree exp, unsigned UIOpc, unsigned SIOpc,
+ unsigned FPOpc, bool isUnord) {
+ // Get the type of the operands
+ tree Op0Ty = TREE_TYPE(TREE_OPERAND(exp,0));
+
+ // Get the compare operands, in the right type. Comparison of struct is not
+ // allowed, so this is safe.
Value *LHS = Emit(TREE_OPERAND(exp, 0), 0);
Value *RHS = Emit(TREE_OPERAND(exp, 1), 0);
RHS = NOOPCastToType(RHS, LHS->getType());
-
assert(LHS->getType() == RHS->getType() && "Binop type equality failure!");
+
+ // Handle the integer/pointer cases
+ if (!FLOAT_TYPE_P(Op0Ty)) {
+ // Determine which predicate to use based on signedness
+ ICmpInst::Predicate pred =
+ ICmpInst::Predicate(TYPE_UNSIGNED(Op0Ty) ? UIOpc : SIOpc);
+
+ // Get the compare instructions
+ Value *Result = new ICmpInst(pred, LHS, RHS, "tmp", CurBB);
+
+ // The GCC type is probably an int, not a bool.
+ return CastToType(Result, ConvertType(TREE_TYPE(exp)));
+ }
+
+ // Handle floating point comparisons, if we get here.
+ if (TREE_CODE(Op0Ty) == COMPLEX_TYPE)
+ return EmitComplexBinOp(exp, 0); // Complex ==/!=
Value *Result = 0;
- if (Opc)
- Result = new SetCondInst((Instruction::BinaryOps)Opc, LHS, RHS, "tmp",
+ if (FPOpc)
+ Result = new SetCondInst((Instruction::BinaryOps)FPOpc, LHS, RHS, "tmp",
CurBB);
if (isUnord) {
@@ -2329,7 +2366,7 @@
Result = IsUnord;
// If this is an ORDERED_EXPR, invert the result of the isunordered call.
- if (Opc == ORDERED_EXPR)
+ if (FPOpc == ORDERED_EXPR)
Result = BinaryOperator::createNot(Result, "tmp", CurBB);
}
@@ -2464,7 +2501,8 @@
return CastToType(Merge, ConvertType(TREE_TYPE(exp)));
}
-Value *TreeToLLVM::EmitMinMaxExpr(tree exp, unsigned CmpOpc) {
+Value *TreeToLLVM::EmitMinMaxExpr(tree exp, unsigned UIPred, unsigned SIPred,
+ unsigned FPOpc) {
Value *LHS = Emit(TREE_OPERAND(exp, 0), 0);
Value *RHS = Emit(TREE_OPERAND(exp, 1), 0);
@@ -2472,9 +2510,16 @@
LHS = NOOPCastToType(LHS, Ty);
RHS = NOOPCastToType(RHS, Ty);
- Value *Pred = new SetCondInst((Instruction::BinaryOps)CmpOpc, LHS, RHS,
- "tmp", CurBB);
- return new SelectInst(Pred, LHS, RHS,
+ Value *Compare;
+ if (LHS->getType()->isFloatingPoint())
+ Compare = new SetCondInst((Instruction::BinaryOps)FPOpc, LHS, RHS,
+ "tmp", CurBB);
+ else if TYPE_UNSIGNED(TREE_TYPE(TREE_OPERAND(exp, 0)))
+ Compare = new ICmpInst(ICmpInst::Predicate(UIPred), LHS, RHS, "tmp", CurBB);
+ else
+ Compare = new ICmpInst(ICmpInst::Predicate(SIPred), LHS, RHS, "tmp", CurBB);
+
+ return new SelectInst(Compare, LHS, RHS,
TREE_CODE(exp) == MAX_EXPR ? "max" : "min", CurBB);
}
Index: gcc/llvm-internal.h
===================================================================
--- gcc/llvm-internal.h (revision 225)
+++ gcc/llvm-internal.h (working copy)
@@ -388,13 +388,15 @@
Value *EmitABS_EXPR(tree_node *exp);
Value *EmitBIT_NOT_EXPR(tree_node *exp);
Value *EmitTRUTH_NOT_EXPR(tree_node *exp);
- Value *EmitCompare(tree_node *exp, unsigned Opc, bool isUnordered);
+ Value *EmitCompare(tree_node *exp, unsigned UIPred, unsigned SIPred,
+ unsigned FPOpc, bool isUnordered);
Value *EmitBinOp(tree_node *exp, Value *DestLoc, unsigned Opc);
Value *EmitPtrBinOp(tree_node *exp, unsigned Opc);
Value *EmitTruthOp(tree_node *exp, unsigned Opc);
Value *EmitShiftOp(tree_node *exp, Value *DestLoc, unsigned Opc);
Value *EmitRotateOp(tree_node *exp, unsigned Opc1, unsigned Opc2);
- Value *EmitMinMaxExpr(tree_node *exp, unsigned Opc);
+ Value *EmitMinMaxExpr(tree_node *exp, unsigned UIPred, unsigned SIPred,
+ unsigned Opc);
// Inline Assembly and Register Variables.
Value *EmitASM_EXPR(tree_node *exp);
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
