All Done.
On Wed, 2006-10-25 at 21:03 -0700, Chris Lattner wrote:
>
> On Oct 25, 2006, at 12:15 PM, Reid Spencer wrote:
>
> > Attached are two patch files to replace the DIV instruction with 3
> > instructions: SDiv, UDiv, FDiv. The first file patches llvm. The
> > second
> > file patches llvm-gcc4.
> >
> >
> > This is the 2nd attempt to provide the patch. All comments are
> > welcome.
>
>
> 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
