<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Thanks for the review! I will address these later today.<div><br><div><div>On Sep 18, 2012, at 7:17 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_extra"><div class="gmail_quote">I'm sad this didn't get any pre-commit review on the public mailing lists...</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Tue, Sep 18, 2012 at 3:02 PM, Michael Ilseman <span dir="ltr"><<a href="mailto:milseman@apple.com" target="_blank" class="cremed">milseman@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: milseman<br>
Date: Tue Sep 18 17:02:40 2012<br>
New Revision: 164173<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=164173&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=164173&view=rev</a><br>
Log:<br>
New utility for expanding integer division for targets that don't support it.<br>
<br>
Implementation derived from compiler-rt's implementation of signed and unsigned integer division.<br></blockquote><div><br></div><div>There are neither users nor tests of this code in the tree. That is a really bad state. This will bitrot immediately. Please write thorough tests for this if you want to keep it in the tree as a utility for out-of-tree projects. Other developers have no other way to exercise it.</div>
<div><br></div><div>Also:</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+namespace llvm {<br>
+<br>
+ bool expandDivision(BinaryOperator* Div);<br></blockquote><div><br></div><div>Zero comments? No Doxygen? This hardly seems appropriate. I realize the external behavior is simple, but that doesn't mean it can go without documentation.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+} // End llvm namespace<br>
+<br>
+#endif<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CMakeLists.txt?rev=164173&r1=164172&r2=164173&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CMakeLists.txt?rev=164173&r1=164172&r2=164173&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/CMakeLists.txt (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/CMakeLists.txt Tue Sep 18 17:02:40 2012<br>
@@ -11,6 +11,7 @@<br>
DemoteRegToStack.cpp<br>
InlineFunction.cpp<br>
InstructionNamer.cpp<br>
+ IntegerDivision.cpp<br>
LCSSA.cpp<br>
Local.cpp<br>
LoopSimplify.cpp<br>
<br>
Added: llvm/trunk/lib/Transforms/Utils/IntegerDivision.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/IntegerDivision.cpp?rev=164173&view=auto" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/IntegerDivision.cpp?rev=164173&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/IntegerDivision.cpp (added)<br>
+++ llvm/trunk/lib/Transforms/Utils/IntegerDivision.cpp Tue Sep 18 17:02:40 2012<br>
@@ -0,0 +1,306 @@<br>
+//===-- IntegerDivision.cpp - Expand integer division ---------------------===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// This file contains an implementation of 32bit scalar integer division for<br>
+// targets that don't have native support. It's largely derived from<br>
+// compiler-rt's implementation of __udivsi3, but hand-tuned to reduce the<br>
+// amount of control flow<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#define DEBUG_TYPE "integer-division"<br>
+#include "llvm/Function.h"<br>
+#include "llvm/Instructions.h"<br>
+#include "llvm/Intrinsics.h"<br>
+#include "llvm/IRBuilder.h"<br>
+#include "llvm/Transforms/Utils/IntegerDivision.h"<br>
+<br>
+using namespace llvm;<br>
+<br>
+// Generate code to divide two signed integers. Returns the quotient, rounded<br>
+// towards 0. Builder's insert point should be pointing at the sdiv<br>
+// instruction. This will generate a udiv in the process, and Builder's insert<br>
+// point will be pointing at the udiv (if present, i.e. not folded), ready to be<br>
+// expanded if the user wishes.<br>
+static Value* GenerateSignedDivisionCode(Value* Dividend, Value* Divisor,<br>
+ IRBuilder<>& Builder) {<br>
+ // Implementation taken from compiler-rt's __divsi3<br>
+<br>
+ ConstantInt* ThirtyOne = Builder.getInt32(31);<br></blockquote><div><br></div><div>The "*" are consistently in the wrong place. Also this doesn't follow the style guide's naming conventions. Also it should have a doxygen comment.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ // ; %tmp = ashr i32 %dividend, 31<br>
+ // ; %tmp1 = ashr i32 %divisor, 31<br>
+ // ; %tmp2 = xor i32 %tmp, %dividend<br>
+ // ; %u_dvnd = sub nsw i32 %tmp2, %tmp<br>
+ // ; %tmp3 = xor i32 %tmp1, %divisor<br>
+ // ; %u_dvsr = sub nsw i32 %tmp3, %tmp1<br>
+ // ; %q_sgn = xor i32 %tmp1, %tmp<br>
+ // ; %q_mag = udiv i32 %u_dvnd, %u_dvsr<br>
+ // ; %tmp4 = xor i32 %q_mag, %q_sgn<br>
+ // ; %q = sub i32 %tmp4, %q_sgn<br>
+ Value* Tmp = Builder.CreateAShr(Dividend, ThirtyOne);<br>
+ Value* Tmp1 = Builder.CreateAShr(Divisor, ThirtyOne);<br>
+ Value* Tmp2 = Builder.CreateXor(Tmp, Dividend);<br>
+ Value* U_Dvnd = Builder.CreateSub(Tmp2, Tmp);<br></blockquote><div><br></div><div>You've given these variables terribly hard to read names when the length of name costs nothing. Could use use something more readable?</div>
<div><br></div><div>Also, You've not given any names to the actual instructions being built, which will make reading the IR produced by this much harder.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ Value* Tmp3 = Builder.CreateXor(Tmp1, Divisor);<br>
+ Value* U_Dvsr = Builder.CreateSub(Tmp3, Tmp1);<br>
+ Value* Q_Sgn = Builder.CreateXor(Tmp1, Tmp);<br>
+ Value* Q_Mag = Builder.CreateUDiv(U_Dvnd, U_Dvsr);<br>
+ Value* Tmp4 = Builder.CreateXor(Q_Mag, Q_Sgn);<br>
+ Value* Q = Builder.CreateSub(Tmp4, Q_Sgn);<br>
+<br>
+ if (Instruction* UDiv = dyn_cast<Instruction>(Q_Mag))<br>
+ Builder.SetInsertPoint(UDiv);<br>
+<br>
+ return Q;<br>
+}<br>
+<br>
+// Generates code to divide two unsigned scalar 32-bit integers. Returns the<br>
+// quotient, rounded towards 0. Builder's insert point should be pointing at the<br>
+// udiv instruction.<br>
+static Value* GenerateUnsignedDivisionCode(Value* Dividend, Value* Divisor,<br>
+ IRBuilder<>& Builder) {<br>
+ // The basic algorithm can be found in the compiler-rt project's<br>
+ // implementation of __udivsi3.c. Here, we do a lower-level IR based approach<br>
+ // that's been hand-tuned to lessen the amount of control flow involved.<br>
+<br>
+ // Some helper values<br>
+ IntegerType* I32Ty = Builder.getInt32Ty();<br>
+<br>
+ ConstantInt* Zero = Builder.getInt32(0);<br>
+ ConstantInt* One = Builder.getInt32(1);<br>
+ ConstantInt* ThirtyOne = Builder.getInt32(31);<br>
+ ConstantInt* NegOne = ConstantInt::getSigned(I32Ty, -1);<br>
+ ConstantInt* True = Builder.getTrue();<br>
+<br>
+ BasicBlock* IBB = Builder.GetInsertBlock();<br>
+ Function* F = IBB->getParent();<br>
+ Function* CTLZi32 = Intrinsic::getDeclaration(F->getParent(), Intrinsic::ctlz,<br>
+ I32Ty);<br>
+<br>
+ // Our CFG is going to look like:<br>
+ // +---------------------+<br>
+ // | special-cases |<br>
+ // | ... |<br>
+ // +---------------------+<br>
+ // | |<br>
+ // | +----------+<br>
+ // | | bb1 |<br>
+ // | | ... |<br>
+ // | +----------+<br>
+ // | | |<br>
+ // | | +------------+<br>
+ // | | | preheader |<br>
+ // | | | ... |<br>
+ // | | +------------+<br>
+ // | | |<br>
+ // | | | +---+<br>
+ // | | | | |<br>
+ // | | +------------+ |<br>
+ // | | | do-while | |<br>
+ // | | | ... | |<br>
+ // | | +------------+ |<br>
+ // | | | | |<br>
+ // | +-----------+ +---+<br>
+ // | | loop-exit |<br>
+ // | | ... |<br>
+ // | +-----------+<br>
+ // | |<br>
+ // +-------+<br>
+ // | ... |<br>
+ // | end |<br>
+ // +-------+<br>
+ BasicBlock* SpecialCases = Builder.GetInsertBlock();<br>
+ SpecialCases->setName(Twine(SpecialCases->getName(), "_udiv-special-cases"));<br>
+ BasicBlock* End = SpecialCases->splitBasicBlock(Builder.GetInsertPoint(),<br>
+ "udiv-end");<br>
+ BasicBlock* LoopExit = BasicBlock::Create(Builder.getContext(),<br>
+ "udiv-loop-exit", F, End);<br>
+ BasicBlock* DoWhile = BasicBlock::Create(Builder.getContext(),<br>
+ "udiv-do-while", F, End);<br>
+ BasicBlock* Preheader = BasicBlock::Create(Builder.getContext(),<br>
+ "udiv-preheader", F, End);<br>
+ BasicBlock* BB1 = BasicBlock::Create(Builder.getContext(),<br>
+ "udiv-bb1", F, End);<br>
+<br>
+ // We'll be overwriting the terminator to insert our extra blocks<br>
+ SpecialCases->getTerminator()->eraseFromParent();<br>
+<br>
+ // First off, check for special cases: dividend or divisor is zero, divisor<br>
+ // is greater than dividend, and divisor is 1.<br>
+ // ; special-cases:<br>
+ // ; %ret0_1 = icmp eq i32 %divisor, 0<br>
+ // ; %ret0_2 = icmp eq i32 %dividend, 0<br>
+ // ; %ret0_3 = or i1 %ret0_1, %ret0_2<br>
+ // ; %tmp0 = tail call i32 @llvm.ctlz.i32(i32 %divisor, i1 true)<br>
+ // ; %tmp1 = tail call i32 @llvm.ctlz.i32(i32 %dividend, i1 true)<br>
+ // ; %sr = sub nsw i32 %tmp0, %tmp1<br>
+ // ; %ret0_4 = icmp ugt i32 %sr, 31<br>
+ // ; %ret0 = or i1 %ret0_3, %ret0_4<br>
+ // ; %retDividend = icmp eq i32 %sr, 31<br>
+ // ; %retVal = select i1 %ret0, i32 0, i32 %dividend<br>
+ // ; %earlyRet = or i1 %ret0, %retDividend<br>
+ // ; br i1 %earlyRet, label %end, label %bb1<br>
+ Builder.SetInsertPoint(SpecialCases);<br>
+ Value* Ret0_1 = Builder.CreateICmpEQ(Divisor, Zero);<br>
+ Value* Ret0_2 = Builder.CreateICmpEQ(Dividend, Zero);<br>
+ Value* Ret0_3 = Builder.CreateOr(Ret0_1, Ret0_2);<br>
+ Value* Tmp0 = Builder.CreateCall2(CTLZi32, Divisor, True);<br>
+ Value* Tmp1 = Builder.CreateCall2(CTLZi32, Dividend, True);<br>
+ Value* SR = Builder.CreateSub(Tmp0, Tmp1);<br>
+ Value* Ret0_4 = Builder.CreateICmpUGT(SR, ThirtyOne);<br>
+ Value* Ret0 = Builder.CreateOr(Ret0_3, Ret0_4);<br>
+ Value* RetDividend = Builder.CreateICmpEQ(SR, ThirtyOne);<br>
+ Value* RetVal = Builder.CreateSelect(Ret0, Zero, Dividend);<br>
+ Value* EarlyRet = Builder.CreateOr(Ret0, RetDividend);<br>
+ Builder.CreateCondBr(EarlyRet, End, BB1);<br>
+<br>
+ // ; bb1: ; preds = %special-cases<br>
+ // ; %sr_1 = add i32 %sr, 1<br>
+ // ; %tmp2 = sub i32 31, %sr<br>
+ // ; %q = shl i32 %dividend, %tmp2<br>
+ // ; %skipLoop = icmp eq i32 %sr_1, 0<br>
+ // ; br i1 %skipLoop, label %loop-exit, label %preheader<br>
+ Builder.SetInsertPoint(BB1);<br>
+ Value* SR_1 = Builder.CreateAdd(SR, One);<br>
+ Value* Tmp2 = Builder.CreateSub(ThirtyOne, SR);<br>
+ Value* Q = Builder.CreateShl(Dividend, Tmp2);<br>
+ Value* SkipLoop = Builder.CreateICmpEQ(SR_1, Zero);<br>
+ Builder.CreateCondBr(SkipLoop, LoopExit, Preheader);<br>
+<br>
+ // ; preheader: ; preds = %bb1<br>
+ // ; %tmp3 = lshr i32 %dividend, %sr_1<br>
+ // ; %tmp4 = add i32 %divisor, -1<br>
+ // ; br label %do-while<br>
+ Builder.SetInsertPoint(Preheader);<br>
+ Value* Tmp3 = Builder.CreateLShr(Dividend, SR_1);<br>
+ Value* Tmp4 = Builder.CreateAdd(Divisor, NegOne);<br>
+ Builder.CreateBr(DoWhile);<br>
+<br>
+ // ; do-while: ; preds = %do-while, %preheader<br>
+ // ; %carry_1 = phi i32 [ 0, %preheader ], [ %carry, %do-while ]<br>
+ // ; %sr_3 = phi i32 [ %sr_1, %preheader ], [ %sr_2, %do-while ]<br>
+ // ; %r_1 = phi i32 [ %tmp3, %preheader ], [ %r, %do-while ]<br>
+ // ; %q_2 = phi i32 [ %q, %preheader ], [ %q_1, %do-while ]<br>
+ // ; %tmp5 = shl i32 %r_1, 1<br>
+ // ; %tmp6 = lshr i32 %q_2, 31<br>
+ // ; %tmp7 = or i32 %tmp5, %tmp6<br>
+ // ; %tmp8 = shl i32 %q_2, 1<br>
+ // ; %q_1 = or i32 %carry_1, %tmp8<br>
+ // ; %tmp9 = sub i32 %tmp4, %tmp7<br>
+ // ; %tmp10 = ashr i32 %tmp9, 31<br>
+ // ; %carry = and i32 %tmp10, 1<br>
+ // ; %tmp11 = and i32 %tmp10, %divisor<br>
+ // ; %r = sub i32 %tmp7, %tmp11<br>
+ // ; %sr_2 = add i32 %sr_3, -1<br>
+ // ; %tmp12 = icmp eq i32 %sr_2, 0<br>
+ // ; br i1 %tmp12, label %loop-exit, label %do-while<br>
+ Builder.SetInsertPoint(DoWhile);<br>
+ PHINode* Carry_1 = Builder.CreatePHI(I32Ty, 2);<br>
+ PHINode* SR_3 = Builder.CreatePHI(I32Ty, 2);<br>
+ PHINode* R_1 = Builder.CreatePHI(I32Ty, 2);<br>
+ PHINode* Q_2 = Builder.CreatePHI(I32Ty, 2);<br>
+ Value* Tmp5 = Builder.CreateShl(R_1, One);<br>
+ Value* Tmp6 = Builder.CreateLShr(Q_2, ThirtyOne);<br>
+ Value* Tmp7 = Builder.CreateOr(Tmp5, Tmp6);<br>
+ Value* Tmp8 = Builder.CreateShl(Q_2, One);<br>
+ Value* Q_1 = Builder.CreateOr(Carry_1, Tmp8);<br>
+ Value* Tmp9 = Builder.CreateSub(Tmp4, Tmp7);<br>
+ Value* Tmp10 = Builder.CreateAShr(Tmp9, 31);<br>
+ Value* Carry = Builder.CreateAnd(Tmp10, One);<br>
+ Value* Tmp11 = Builder.CreateAnd(Tmp10, Divisor);<br>
+ Value* R = Builder.CreateSub(Tmp7, Tmp11);<br>
+ Value* SR_2 = Builder.CreateAdd(SR_3, NegOne);<br>
+ Value* Tmp12 = Builder.CreateICmpEQ(SR_2, Zero);<br>
+ Builder.CreateCondBr(Tmp12, LoopExit, DoWhile);<br>
+<br>
+ // ; loop-exit: ; preds = %do-while, %bb1<br>
+ // ; %carry_2 = phi i32 [ 0, %bb1 ], [ %carry, %do-while ]<br>
+ // ; %q_3 = phi i32 [ %q, %bb1 ], [ %q_1, %do-while ]<br>
+ // ; %tmp13 = shl i32 %q_3, 1<br>
+ // ; %q_4 = or i32 %carry_2, %tmp13<br>
+ // ; br label %end<br>
+ Builder.SetInsertPoint(LoopExit);<br>
+ PHINode* Carry_2 = Builder.CreatePHI(I32Ty, 2);<br>
+ PHINode* Q_3 = Builder.CreatePHI(I32Ty, 2);<br>
+ Value* Tmp13 = Builder.CreateShl(Q_3, One);<br>
+ Value* Q_4 = Builder.CreateOr(Carry_2, Tmp13);<br>
+ Builder.CreateBr(End);<br>
+<br>
+ // ; end: ; preds = %loop-exit, %special-cases<br>
+ // ; %q_5 = phi i32 [ %q_4, %loop-exit ], [ %retVal, %special-cases ]<br>
+ // ; ret i32 %q_5<br>
+ Builder.SetInsertPoint(End, End->begin());<br>
+ PHINode* Q_5 = Builder.CreatePHI(I32Ty, 2);<br>
+<br>
+ // Populate the Phis, since all values have now been created. Our Phis were:<br>
+ // ; %carry_1 = phi i32 [ 0, %preheader ], [ %carry, %do-while ]<br>
+ Carry_1->addIncoming(Zero, Preheader);<br>
+ Carry_1->addIncoming(Carry, DoWhile);<br>
+ // ; %sr_3 = phi i32 [ %sr_1, %preheader ], [ %sr_2, %do-while ]<br>
+ SR_3->addIncoming(SR_1, Preheader);<br>
+ SR_3->addIncoming(SR_2, DoWhile);<br>
+ // ; %r_1 = phi i32 [ %tmp3, %preheader ], [ %r, %do-while ]<br>
+ R_1->addIncoming(Tmp3, Preheader);<br>
+ R_1->addIncoming(R, DoWhile);<br>
+ // ; %q_2 = phi i32 [ %q, %preheader ], [ %q_1, %do-while ]<br>
+ Q_2->addIncoming(Q, Preheader);<br>
+ Q_2->addIncoming(Q_1, DoWhile);<br>
+ // ; %carry_2 = phi i32 [ 0, %bb1 ], [ %carry, %do-while ]<br>
+ Carry_2->addIncoming(Zero, BB1);<br>
+ Carry_2->addIncoming(Carry, DoWhile);<br>
+ // ; %q_3 = phi i32 [ %q, %bb1 ], [ %q_1, %do-while ]<br>
+ Q_3->addIncoming(Q, BB1);<br>
+ Q_3->addIncoming(Q_1, DoWhile);<br>
+ // ; %q_5 = phi i32 [ %q_4, %loop-exit ], [ %retVal, %special-cases ]<br>
+ Q_5->addIncoming(Q_4, LoopExit);<br>
+ Q_5->addIncoming(RetVal, SpecialCases);<br>
+<br>
+ return Q_5;<br>
+}<br>
+<br>
+bool llvm::expandDivision(BinaryOperator* Div) {<br>
+ assert(Div->getOpcode() == Instruction::SDiv ||<br>
+ Div->getOpcode() == Instruction::UDiv<br>
+ && "Trying to expand division from a non-division function");<br>
+<br>
+ IRBuilder<> Builder(Div);<br>
+<br>
+ if (Div->getType()->isVectorTy()) {<br>
+ assert(0 && "Div over vectors not supported");<br>
+ return false;<br>
+ }<br>
+<br>
+ // First prepare the sign if it's a signed division<br>
+ if (Div->getOpcode() == Instruction::SDiv) {<br>
+ // Lower the code to unsigned division, and reset Div to point to the udiv.<br>
+ Value* Quotient = GenerateSignedDivisionCode(Div->getOperand(0),<br>
+ Div->getOperand(1), Builder);<br>
+ Div->replaceAllUsesWith(Quotient);<br>
+ Div->dropAllReferences();<br>
+ Div->eraseFromParent();<br>
+<br>
+ // If we didn't actually generate a udiv instruction, we're done<br>
+ BinaryOperator* BO = dyn_cast<BinaryOperator>(Builder.GetInsertPoint());<br>
+ if (!BO || BO->getOpcode() != Instruction::UDiv)<br>
+ return true;<br>
+<br>
+ Div = BO;<br>
+ }<br>
+<br>
+ // Insert the unsigned division code<br>
+ Value* Quotient = GenerateUnsignedDivisionCode(Div->getOperand(0),<br>
+ Div->getOperand(1),<br>
+ Builder);<br>
+ Div->replaceAllUsesWith(Quotient);<br>
+ Div->dropAllReferences();<br>
+ Div->eraseFromParent();<br>
+<br>
+ return true;<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</blockquote></div><br></div></body></html>