On Oct 25, 2006, at 12:15 PM, Reid Spencer wrote:
This patch is *far* improved over the last one. Nit picky stuff below. Please make these changes, but there is no need to repost these diffs for review. llvm part, without instcombine: +// This function is used to obtain the correct opcode for an instruction when +// an obsolete opcode is encountered. The OI parameter (OpcodeInfo) has both +// an opcode and an "obsolete" flag. These are generated by the lexer and +// the "obsolete" member will be true when the lexer encounters the token for +// an obsolete opcode. For example, "div" was replaced by [usf]div but we need +// to maintain backwards compatibility for asm files that still have the "div" +// instruction. This function handles converting div -> [usf]div appropriately. +static void +sanitizeOpCode(OpcodeInfo<Instruction::BinaryOps> &OI, const PATypeHolder& PATy) +{ Please convert this to a doxygen comment. Please change the second argument to "const Type *Ty". Bytecode/Reader.cpp: + // If this is a bytecode format that did not include the unreachable + // instruction, bump up the opcode number to adjust it. + if (hasNoUnreachableInst) { + if (Opcode >= Instruction::Unreachable && + Opcode < 62) { // 62 + ++Opcode; + } + } What is the "// 62" comment? + case 11: // Rem + // As with "Div", make the signed/unsigned Rem instruction choice based + // on the type of the instruction. + if (ArgVec[0]->getType()->isFloatingPoint()) + Opcode = Instruction::Rem; + else if (ArgVec[0]->getType()->isSigned()) + Opcode = Instruction::Rem; + else + Opcode = Instruction::Rem; Heh, so forward looking :), no need to change it. + // In version 5 and prior, the integer types were distinguished by sign. + // That is we have UIntTy and IntTy as well as ConstantSInt and + // ConstantUInt. In version 6, the integer types became signless so we + // need to note that we have signed integer types in prior versions. + bool hasSignedIntegers; This is set but never checked, please remove it. diff -t -d -u -p -5 -r1.94 ConstantFolding.cpp --- lib/VMCore/ConstantFolding.cpp 20 Oct 2006 07:07:24 -0000 1.94 +++ lib/VMCore/ConstantFolding.cpp 25 Oct 2006 18:51:19 -0000 @@ -38,11 +38,13 @@ namespace { ... + static Constant *UDiv(const ConstantInt *V1, const ConstantInt *V2) { + if (V2->isNullValue()) + return 0; if (V2->isAllOnesValue() && // MIN_INT / -1 (BuiltinType)V1->getZExtValue() == -(BuiltinType)V1->getZExtValue()) return 0; + BuiltinType R = (BuiltinType)(V1->getZExtValue() / V2->getZExtValue()); + return ConstantInt::get(*Ty, R); + } This check: if (V2->isAllOnesValue() && // MIN_INT / -1 (BuiltinType)V1->getZExtValue() == -(BuiltinType)V1->getZExtValue()) return 0; Is not needed in the udiv case, it is over-conservative (yes, the original code was over-conservative in the same way). -Chris |
_______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
