On Nov 18, 2006, at 11:36 PM, Reid Spencer wrote:
This is the first in a series of patches to split the SETCC
instructions
into just two instructions: icmp (integer compare) and fcmp (floating
point compare). This patch is a no-op. It introduces code that will be
used in subsequent patches. The new instruction codes are reserved,
the
instruction classes are defined, and the InstVisitor class (and users)
is updated to recognize the new instructions. However, nothing else in
LLVM will produce these instructions (yet). The patch passes all
tests.
The comments on fcmpinst are not correct in some cases:
+ /// For example, EQ -> NE, UGT -> ULE, SLT -> SGE, etc.
+ /// @returns the inverse predicate for the instructions current
predicate.
+ /// @brief Return the inverse of the predicate
These refer to SLT which isn't valid for fcmp, similarly for others.
--- include/llvm/Support/InstVisitor.h 9 Oct 2006 18:33:08 -0000 1.41
+++ include/llvm/Support/InstVisitor.h 19 Nov 2006 07:32:51 -0000
@@ -167,10 +167,12 @@ public:
RetTy visitSwitchInst(SwitchInst &I) { DELEGATE
(TerminatorInst);}
RetTy visitInvokeInst(InvokeInst &I) { DELEGATE
(TerminatorInst);}
RetTy visitUnwindInst(UnwindInst &I) { DELEGATE
(TerminatorInst);}
RetTy visitUnreachableInst(UnreachableInst &I) { DELEGATE
(TerminatorInst);}
RetTy visitSetCondInst(SetCondInst &I) { DELEGATE
(BinaryOperator);}
+ RetTy visitICmpInst(ICmpInst &I) { DELEGATE
(Instruction);}
+ RetTy visitFCmpInst(FCmpInst &I) { DELEGATE
(Instruction);}
RetTy visitMallocInst(MallocInst &I) { DELEGATE
(AllocationInst);}
RetTy visitAllocaInst(AllocaInst &I) { DELEGATE
(AllocationInst);}
RetTy visitFreeInst(FreeInst &I) { DELEGATE
(Instruction); }
RetTy visitLoadInst(LoadInst &I) { DELEGATE
(Instruction); }
RetTy visitStoreInst(StoreInst &I) { DELEGATE
(Instruction); }
This should delegate to CmpInst, which delegates to Instruction.
+void CmpInst::swapOperands() {
+ if (ICmpInst* IC = dyn_cast<ICmpInst>(this)) {
+ IC->swapOperands();
+ } else if (FCmpInst* FC = dyn_cast<FCmpInst>(this)) {
+ FC->swapOperands();
+ }
+ assert(!"Unknown CmpInst type");
+}
I'd write this as:
+void CmpInst::swapOperands() {
+ if (ICmpInst* IC = dyn_cast<ICmpInst>(this)) {
+ IC->swapOperands();
+ } else {
+ cast<FCmpInst>(this)->swapOperands();
+ }
which is faster in release builds. Besides, the assert will always
trigger above. Please
use 0 && "foo", which is the standard idiom instead of !"foo".
+FCmpInst::Predicate FCmpInst::getInversePredicate(Predicate pred) {
+ switch (pred) {
+ default:
+ assert(!"Unknown icmp predicate!");
+ case OEQ: return ONE;
+ case ONE: return OEQ;
+ case OGT: return OLE;
+ case OLT: return OGE;
+ case OGE: return OLT;
+ case OLE: return OGT;
+ case UEQ: return UNE;
+ case UNE: return UEQ;
+ case UGT: return ULE;
+ case ULT: return UGE;
+ case UGE: return ULT;
+ case ULE: return UGT;
+ case TRUE: return FALSE;
+ case FALSE: return TRUE;
+ }
+}
These aren't right, for example, !OEQ = UNE
+void Verifier::visitICmpInst(ICmpInst& IC) {
+ // Check that icmp instructions return bool
+ Assert1(IC.getType() == Type::BoolTy,
+ "ICmp instructions must return boolean values!", &IC);
+ // Check that the operands are the same type
+ const Type* Op0Ty = IC.getOperand(0)->getType();
+ const Type* Op1Ty = IC.getOperand(0)->getType();
+ Assert1(Op0Ty == Op1Ty,
+ "Both operands to ICmp instruction are not of the same
type!", &IC);
+ // Check that the operands are the right type
+ Assert1(Op0Ty->isIntegral() || Op0Ty->getTypeID() ==
Type::PointerTyID ||
+ (isa<PackedType>(Op0Ty) &&
+ cast<PackedType>(Op0Ty)->getElementType()->isIntegral()),
+ "Invalid operand types for ICmp instruction", &IC);
Most of these check should also happen in the ctors. The check for
the result of bool should just be in the ctor. Once an instruction
is created, you can't change its result type.
+void BVNImpl::visitCmpInst(CmpInst &CI1) {
+ Value *LHS = CI1.getOperand(0);
+ for (Value::use_iterator UI = LHS->use_begin(), UE = LHS->use_end();
+ UI != UE; ++UI)
+ if (CmpInst *CI2 = dyn_cast<CmpInst>(*UI))
+ // Check to see if this comparinstruction is not CI, but same
opcode,
+ // same predicate, and in the same function.
typo comparinstruction
+ // And its commutative (equal or not equal)
+ ((isa<ICmpInst>(CI1) && cast<ICmpInst>
(CI1).isCommutative()) ||
+ (isa<FCmpInst>(CI1) && cast<ICmpInst>
(CI1).isCommutative()))))
The second line has a typo (ICmp -> FCmp). The second line begs for
a CmpInst::isCommutative()
Go ahead and commit after tweaking these,
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
