[llvm-commits] [llvm] r55969 - in /llvm/trunk: docs/LangRef.html include/llvm/Bitcode/LLVMBitCodes.h include/llvm/InstrTypes.h include/llvm/Instructions.h lib/AsmParser/llvmAsmParser.y lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Writer/BitcodeWriter.cpp lib/VMCore/Verifier.cpp test/Assembler/vbool-cmp.ll test/Assembler/vector-select.ll
Chris Lattner
clattner at apple.com
Sun Sep 21 23:26:17 PDT 2008
On Sep 8, 2008, at 6:02 PM, Dan Gohman wrote:
> Author: djg
> Date: Mon Sep 8 20:02:47 2008
> New Revision: 55969
>
> URL: http://llvm.org/viewvc/llvm-project?rev=55969&view=rev
> Log:
> Extend the vcmp/fcmp LLVM IR instructions to take vectors as arguments
> and, if so, to return a vector of boolean as a result;
Hi Dan and Preston,
I thought the idea here was to change vicmp and vfcmp to have this
behavior, not change icmp/fcmp to do this. The problem with changing
icmp/fcmp is that you'll break a lot of assumptions throughout the
optimizer that the result of icmp/fcmp is an i1, and the code to
handle this has already been updated for v*cmp.
>
>> The FUNC_CODE_INST_VCMP thing is a little counter-intuitive. I guess
>> it's necessary in order to remain compatible with exiting vector
>> FUNC_CODE_INST_CMP usage though, right?
>
> Correct. Since FUNC_CODE_INST_CMP with vector arguments is used for
> the
> vfcmp and vicmp instructions, it was necessary to add
> FUNC_CODE_INST_VCMP to support the vector forms of fcmp/icmp. It would
> probably have made more sense in the bitcode reader/writer to switch
> vicmp/vfcmp to use a newly defined bitcode operator, but I did not
> because it seemed likely that it would break existing code.
Is there any reason to keep around the old v*cmp behavior of returning
a vector of integers? If not, please remove them (as soon as there
are no regressions from doing this). I don't want to have two
different vector compare implementations floating around.
Also, LLVM 2.4 is coming up very soon now, would you be ok with
deferring (and reverting) these changes until after 2.4 branches in a
couple weeks? If you'd prefer to do development on a branch until
then that would be fine of course.
> Extend the select LLVM IR instruction to allow you to specify a result
> type which is a vector of boolean, in which case the result will be an
> element-wise selection instead of choosing one vector or the other;
Cool, this is something we've needed for quite awhile.
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h (original)
> +++ llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h Mon Sep 8
> 20:02:47 2008
> @@ -205,7 +205,9 @@
> // FIXME: Remove GETRESULT in favor of EXTRACTVAL in LLVM 3.0
> FUNC_CODE_INST_GETRESULT = 25, // GETRESULT: [ty, opval, n]
> FUNC_CODE_INST_EXTRACTVAL = 26, // EXTRACTVAL: [n x operands]
> - FUNC_CODE_INST_INSERTVAL = 27 // INSERTVAL: [n x operands]
> + FUNC_CODE_INST_INSERTVAL = 27, // INSERTVAL: [n x operands]
> + // fcmp/icmp returning vector of Int1Ty, NOT for vicmp/vfcmp
> + FUNC_CODE_INST_VCMP = 28 // VCMP: [opty, opval,
> opval, pred]
Ewww. I still think that vcmp should change into this behavior, if
there is some reason why this would not work, please pick a better
name than INST_VCMP for this!
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/include/llvm/InstrTypes.h (original)
> +++ llvm/trunk/include/llvm/InstrTypes.h Mon Sep 8 20:02:47 2008
> @@ -18,6 +18,7 @@
>
> #include "llvm/Instruction.h"
> #include "llvm/OperandTraits.h"
> +#include "llvm/DerivedTypes.h"
Please move the method out of line, to avoid this #include.
> @@ -732,6 +733,13 @@
> static inline bool classof(const Value *V) {
> return isa<Instruction>(V) && classof(cast<Instruction>(V));
> }
> + /// @brief Create a result type for fcmp/icmp (but not vicmp/vfcmp)
> + static const Type* makeCmpResultType(const Type* opnd_type) {
Instead of 'makeCmp...' please call this 'getVCmpResultType' or
something like that. We use 'get' for uniqued objects like types.
Assuming that you're going to eliminate vicmp/vfcmp, this should be
removed and you should just change its methods.
Okay, I can't review the rest of this patch without getting more
closure on the direction we're going with the IR. I think that
smooshing this into icmp/fcmp is a real mistake. We should change the
old v[if]cmp instruction to return a vector of bool and leave icmp/
fcmp alone. What do you guys think?
If you agree, please revert these patches for now.
-Chris
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/include/llvm/Instructions.h (original)
> +++ llvm/trunk/include/llvm/Instructions.h Mon Sep 8 20:02:47 2008
> @@ -621,7 +621,8 @@
> Value *RHS, ///< The right-hand-side of the expression
> const std::string &NameStr = "", ///< Name of the instruction
> Instruction *InsertBefore = 0 ///< Where to insert
> - ) : CmpInst(Type::Int1Ty, Instruction::ICmp, pred, LHS, RHS,
> NameStr,
> + ) : CmpInst(makeCmpResultType(LHS->getType()),
> + Instruction::ICmp, pred, LHS, RHS, NameStr,
> InsertBefore) {
> assert(pred >= CmpInst::FIRST_ICMP_PREDICATE &&
> pred <= CmpInst::LAST_ICMP_PREDICATE &&
> @@ -629,7 +630,7 @@
> assert(getOperand(0)->getType() == getOperand(1)->getType() &&
> "Both operands to ICmp instruction are not of the same
> type!");
> // Check that the operands are the right type
> - assert((getOperand(0)->getType()->isInteger() ||
> + assert((getOperand(0)->getType()->isIntOrIntVector() ||
> isa<PointerType>(getOperand(0)->getType())) &&
> "Invalid operand types for ICmp instruction");
> }
> @@ -641,7 +642,8 @@
> Value *RHS, ///< The right-hand-side of the expression
> const std::string &NameStr, ///< Name of the instruction
> BasicBlock *InsertAtEnd ///< Block to insert into.
> - ) : CmpInst(Type::Int1Ty, Instruction::ICmp, pred, LHS, RHS,
> NameStr,
> + ) : CmpInst(makeCmpResultType(LHS->getType()),
> + Instruction::ICmp, pred, LHS, RHS, NameStr,
> InsertAtEnd) {
> assert(pred >= CmpInst::FIRST_ICMP_PREDICATE &&
> pred <= CmpInst::LAST_ICMP_PREDICATE &&
> @@ -649,7 +651,7 @@
> assert(getOperand(0)->getType() == getOperand(1)->getType() &&
> "Both operands to ICmp instruction are not of the same
> type!");
> // Check that the operands are the right type
> - assert((getOperand(0)->getType()->isInteger() ||
> + assert((getOperand(0)->getType()->isIntOrIntVector() ||
> isa<PointerType>(getOperand(0)->getType())) &&
> "Invalid operand types for ICmp instruction");
> }
> @@ -754,6 +756,7 @@
> static inline bool classof(const Value *V) {
> return isa<Instruction>(V) && classof(cast<Instruction>(V));
> }
> +
> };
>
> //
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> @@ -773,14 +776,15 @@
> Value *RHS, ///< The right-hand-side of the expression
> const std::string &NameStr = "", ///< Name of the instruction
> Instruction *InsertBefore = 0 ///< Where to insert
> - ) : CmpInst(Type::Int1Ty, Instruction::FCmp, pred, LHS, RHS,
> NameStr,
> + ) : CmpInst(makeCmpResultType(LHS->getType()),
> + Instruction::FCmp, pred, LHS, RHS, NameStr,
> InsertBefore) {
> assert(pred <= FCmpInst::LAST_FCMP_PREDICATE &&
> "Invalid FCmp predicate value");
> assert(getOperand(0)->getType() == getOperand(1)->getType() &&
> "Both operands to FCmp instruction are not of the same
> type!");
> // Check that the operands are the right type
> - assert(getOperand(0)->getType()->isFloatingPoint() &&
> + assert(getOperand(0)->getType()->isFPOrFPVector() &&
> "Invalid operand types for FCmp instruction");
> }
>
> @@ -791,14 +795,15 @@
> Value *RHS, ///< The right-hand-side of the expression
> const std::string &NameStr, ///< Name of the instruction
> BasicBlock *InsertAtEnd ///< Block to insert into.
> - ) : CmpInst(Type::Int1Ty, Instruction::FCmp, pred, LHS, RHS,
> NameStr,
> + ) : CmpInst(makeCmpResultType(LHS->getType()),
> + Instruction::FCmp, pred, LHS, RHS, NameStr,
> InsertAtEnd) {
> assert(pred <= FCmpInst::LAST_FCMP_PREDICATE &&
> "Invalid FCmp predicate value");
> assert(getOperand(0)->getType() == getOperand(1)->getType() &&
> "Both operands to FCmp instruction are not of the same
> type!");
> // Check that the operands are the right type
> - assert(getOperand(0)->getType()->isFloatingPoint() &&
> + assert(getOperand(0)->getType()->isFPOrFPVector() &&
> "Invalid operand types for FCmp instruction");
> }
>
> @@ -837,6 +842,7 @@
> static inline bool classof(const Value *V) {
> return isa<Instruction>(V) && classof(cast<Instruction>(V));
> }
> +
> };
>
> //
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
>
> Modified: llvm/trunk/lib/AsmParser/llvmAsmParser.y
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/llvmAsmParser.y?rev=55969&r1=55968&r2=55969&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/AsmParser/llvmAsmParser.y (original)
> +++ llvm/trunk/lib/AsmParser/llvmAsmParser.y Mon Sep 8 20:02:47 2008
> @@ -1102,7 +1102,7 @@
> %token <BinaryOpVal> ADD SUB MUL UDIV SDIV FDIV UREM SREM FREM AND
> OR XOR
> %token <BinaryOpVal> SHL LSHR ASHR
>
> -%token <OtherOpVal> ICMP FCMP VICMP VFCMP
> +%token <OtherOpVal> ICMP FCMP VICMP VFCMP
> %type <IPredicate> IPredicates
> %type <FPredicate> FPredicates
> %token EQ NE SLT SGT SLE SGE ULT UGT ULE UGE
> @@ -3097,8 +3097,6 @@
> | ICMP IPredicates Types ValueRef ',' ValueRef {
> if (!UpRefs.empty())
> GEN_ERROR("Invalid upreference in type: " + (*$3)-
> >getDescription());
> - if (isa<VectorType>((*$3).get()))
> - GEN_ERROR("Vector types not supported by icmp instruction");
> Value* tmpVal1 = getVal(*$3, $4);
> CHECK_FOR_ERROR
> Value* tmpVal2 = getVal(*$3, $6);
> @@ -3111,8 +3109,6 @@
> | FCMP FPredicates Types ValueRef ',' ValueRef {
> if (!UpRefs.empty())
> GEN_ERROR("Invalid upreference in type: " + (*$3)-
> >getDescription());
> - if (isa<VectorType>((*$3).get()))
> - GEN_ERROR("Vector types not supported by fcmp instruction");
> Value* tmpVal1 = getVal(*$3, $4);
> CHECK_FOR_ERROR
> Value* tmpVal2 = getVal(*$3, $6);
> @@ -3133,7 +3129,7 @@
> CHECK_FOR_ERROR
> $$ = CmpInst::Create($1, $2, tmpVal1, tmpVal2);
> if ($$ == 0)
> - GEN_ERROR("icmp operator returned null");
> + GEN_ERROR("vicmp operator returned null");
> delete $3;
> }
> | VFCMP FPredicates Types ValueRef ',' ValueRef {
> @@ -3147,7 +3143,7 @@
> CHECK_FOR_ERROR
> $$ = CmpInst::Create($1, $2, tmpVal1, tmpVal2);
> if ($$ == 0)
> - GEN_ERROR("fcmp operator returned null");
> + GEN_ERROR("vfcmp operator returned null");
> delete $3;
> }
> | CastOps ResolvedVal TO Types {
> @@ -3163,10 +3159,23 @@
> delete $4;
> }
> | SELECT ResolvedVal ',' ResolvedVal ',' ResolvedVal {
> - if ($2->getType() != Type::Int1Ty)
> - GEN_ERROR("select condition must be boolean");
> + if (isa<VectorType>($2->getType())) {
> + // vector select
> + if (!isa<VectorType>($4->getType())
> + || !isa<VectorType>($6->getType()) )
> + GEN_ERROR("vector select value types must be vector types");
> + const VectorType* cond_type = cast<VectorType>($2->getType());
> + const VectorType* select_type = cast<VectorType>($4-
> >getType());
> + if (cond_type->getElementType() != Type::Int1Ty)
> + GEN_ERROR("vector select condition element type must be
> boolean");
> + if (cond_type->getNumElements() != select_type-
> >getNumElements())
> + GEN_ERROR("vector select number of elements must be the
> same");
> + } else {
> + if ($2->getType() != Type::Int1Ty)
> + GEN_ERROR("select condition must be boolean");
> + }
> if ($4->getType() != $6->getType())
> - GEN_ERROR("select value types should match");
> + GEN_ERROR("select value types must match");
> $$ = SelectInst::Create($2, $4, $6);
> CHECK_FOR_ERROR
> }
>
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=55969&r1=55968&r2=55969&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Mon Sep 8
> 20:02:47 2008
> @@ -1499,8 +1499,19 @@
> Value *TrueVal, *FalseVal, *Cond;
> if (getValueTypePair(Record, OpNum, NextValueNo, TrueVal) ||
> getValue(Record, OpNum, TrueVal->getType(), FalseVal) ||
> - getValue(Record, OpNum, Type::Int1Ty, Cond))
> + getValue(Record, OpNum, 0 /*skip type check*/, Cond))
> return Error("Invalid SELECT record");
> +
> + // select condition can be either i1 or [N x i1]
> + if (const VectorType* vector_type = dyn_cast<const
> VectorType>(Cond->getType())) {
> + // expect <n x i1>
> + if (vector_type->getElementType() != Type::Int1Ty)
> + return Error("Invalid SELECT condition type");
> + } else {
> + // expect i1
> + if (Cond->getType() != Type::Int1Ty)
> + return Error("Invalid SELECT condition type");
> + }
>
> I = SelectInst::Create(Cond, TrueVal, FalseVal);
> break;
> @@ -1563,6 +1574,22 @@
> I = new VICmpInst((ICmpInst::Predicate)Record[OpNum], LHS,
> RHS);
> break;
> }
> + case bitc::FUNC_CODE_INST_VCMP: { // VCMP: [opty, opval, opval,
> pred]
> + // Fcmp/ICmp returning vector of bool
> + unsigned OpNum = 0;
> + Value *LHS, *RHS;
> + if (getValueTypePair(Record, OpNum, NextValueNo, LHS) ||
> + getValue(Record, OpNum, LHS->getType(), RHS) ||
> + OpNum+1 != Record.size())
> + return Error("Invalid VCMP record");
> +
> + // will always be vector
> + if (LHS->getType()->isFPOrFPVector())
> + I = new FCmpInst((FCmpInst::Predicate)Record[OpNum], LHS,
> RHS);
> + else
> + I = new ICmpInst((ICmpInst::Predicate)Record[OpNum], LHS,
> RHS);
> + break;
> + }
> case bitc::FUNC_CODE_INST_GETRESULT: { // GETRESULT: [ty, val, n]
> if (Record.size() != 2)
> return Error("Invalid GETRESULT record");
>
> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=55969&r1=55968&r2=55969&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Mon Sep 8
> 20:02:47 2008
> @@ -641,7 +641,14 @@
> case Instruction::FCmp:
> case Instruction::VICmp:
> case Instruction::VFCmp:
> - Code = bitc::CST_CODE_CE_CMP;
> + if (isa<VectorType>(C->getOperand(0)->getType())
> + && (CE->getOpcode() == Instruction::ICmp
> + || CE->getOpcode() == Instruction::FCmp)) {
> + // compare returning vector of Int1Ty
> + assert(0 && "Unsupported constant!");
> + } else {
> + Code = bitc::CST_CODE_CE_CMP;
> + }
> Record.push_back(VE.getTypeID(C->getOperand(0)->getType()));
> Record.push_back(VE.getValueID(C->getOperand(0)));
> Record.push_back(VE.getValueID(C->getOperand(1)));
> @@ -765,7 +772,14 @@
> case Instruction::FCmp:
> case Instruction::VICmp:
> case Instruction::VFCmp:
> - Code = bitc::FUNC_CODE_INST_CMP;
> + if (isa<VectorType>(I.getOperand(0)->getType())
> + && (I.getOpcode() == Instruction::ICmp
> + || I.getOpcode() == Instruction::FCmp)) {
> + // compare returning vector of Int1Ty
> + Code = bitc::FUNC_CODE_INST_VCMP;
> + } else {
> + Code = bitc::FUNC_CODE_INST_CMP;
> + }
> PushValueAndType(I.getOperand(0), InstID, Vals, VE);
> Vals.push_back(VE.getValueID(I.getOperand(1)));
> Vals.push_back(cast<CmpInst>(I).getPredicate());
>
> Modified: llvm/trunk/lib/VMCore/Verifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Verifier.cpp?rev=55969&r1=55968&r2=55969&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/VMCore/Verifier.cpp (original)
> +++ llvm/trunk/lib/VMCore/Verifier.cpp Mon Sep 8 20:02:47 2008
> @@ -659,8 +659,21 @@
> }
>
> void Verifier::visitSelectInst(SelectInst &SI) {
> - Assert1(SI.getCondition()->getType() == Type::Int1Ty,
> - "Select condition type must be bool!", &SI);
> + if (const VectorType* vt
> + = dyn_cast<VectorType>(SI.getCondition()->getType())) {
> + Assert1( vt->getElementType() == Type::Int1Ty,
> + "Select condition type must be vector of bool!", &SI);
> + if (const VectorType* val_vt
> + = dyn_cast<VectorType>(SI.getTrueValue()->getType())) {
> + Assert1( vt->getNumElements() == val_vt->getNumElements(),
> + "Select vector size != value vector size", &SI);
> + } else {
> + Assert1(0, "Vector select values must have vector types", &SI);
> + }
> + } else {
> + Assert1(SI.getCondition()->getType() == Type::Int1Ty,
> + "Select condition type must be bool!", &SI);
> + }
> Assert1(SI.getTrueValue()->getType() == SI.getFalseValue()-
> >getType(),
> "Select values must have identical types!", &SI);
> Assert1(SI.getTrueValue()->getType() == SI.getType(),
> @@ -1028,7 +1041,7 @@
> 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->isInteger() || isa<PointerType>(Op0Ty),
> + Assert1(Op0Ty->isIntOrIntVector() || isa<PointerType>(Op0Ty),
> "Invalid operand types for ICmp instruction", &IC);
> visitInstruction(IC);
> }
> @@ -1040,7 +1053,7 @@
> Assert1(Op0Ty == Op1Ty,
> "Both operands to FCmp instruction are not of the same
> type!", &FC);
> // Check that the operands are the right type
> - Assert1(Op0Ty->isFloatingPoint(),
> + Assert1(Op0Ty->isFPOrFPVector(),
> "Invalid operand types for FCmp instruction", &FC);
> visitInstruction(FC);
> }
>
> Added: llvm/trunk/test/Assembler/vbool-cmp.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/vbool-cmp.ll?rev=55969&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/test/Assembler/vbool-cmp.ll (added)
> +++ llvm/trunk/test/Assembler/vbool-cmp.ll Mon Sep 8 20:02:47 2008
> @@ -0,0 +1,15 @@
> +; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | grep {icmp slt}
> +; rudimentary test of fcmp/icmp on vectors returning vector of bool
> +
> +define <4 x i1> @ffoo(<4 x float> %a, <4 x float> %b) nounwind {
> +entry:
> + %cmp = fcmp olt <4 x float> %a, %b ; <4 x i1> [#uses=1]
> + ret <4 x i1> %cmp
> +}
> +
> +define <4 x i1> @ifoo(<4 x i32> %a, <4 x i32> %b) nounwind {
> +entry:
> + %cmp = icmp slt <4 x i32> %a, %b ; <4 x i1> [#uses=1]
> + ret <4 x i1> %cmp
> +}
> +
>
> Added: llvm/trunk/test/Assembler/vector-select.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/vector-select.ll?rev=55969&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/test/Assembler/vector-select.ll (added)
> +++ llvm/trunk/test/Assembler/vector-select.ll Mon Sep 8 20:02:47
> 2008
> @@ -0,0 +1,11 @@
> +; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | grep select
> +; rudimentary test of select on vectors returning vector of bool
> +
> +define <4 x i32> @foo(<4 x i32> %a, <4 x i32> %b,
> + <4 x i1> %cond) nounwind {
> +entry:
> + %cmp = select <4 x i1> %cond, <4 x i32> %a, <4 x i32> %b
> + ; <4 x i32> [#uses=1]
> + ret <4 x i32> %cmp
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list