<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>+bool IntegerDivision::runOnFunction(Function &F) {</div><div>+</div><div>+ if (TM)</div><div>+ TLI = TM->getSubtargetImpl()->getTargetLowering();</div><div>+</div><div>+ for (Function::iterator BBI = F.begin(), BBE = F.end(); BBI != BBE; ++BBI) {</div><div>+ BasicBlock *BB = BBI;</div><div>+ for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE; ++II) {</div><div>+ Instruction *I = II;</div><div>+ if (visit(*I)) {</div><div>+ BBI = F.begin();</div><div>+ break;</div><div>+ }</div><div>+ }</div><div>+ }</div><div>+</div><div>+ return false;</div><div>+}</div><div><br></div><div>You should have a bool for whether the code changed, and return that[1]. It’s probably worth building up a worklist of basic blocks up front, to avoid the repeated visiting of already-handled blocks. Also, we know that many of the blocks that are generated from division-expansion would not themselves have rem or div in them.</div><div><br></div><div><br></div><div><div>+bool IntegerDivision::shouldExpandDivRem(const BinaryOperator &I) {</div><div>+ return TLI && TLI->shouldExpandDivRemInIR(I);</div><div>+}</div></div><div><br></div><div>You could do the TLI-is-not-null check in runOnFunction, which is a good place to bail early. Thereafter you can have an assert. I don’t know who would be using this pass normally, though.</div><div><br></div><div>Otherwise the patch LGTM. What is your use case of wanting a full-fledged pass for this? If you have other codegen-prepare-like things to do to the IR, it seems suboptimal to do a pass over every instruction just for div/rem, at least from a low-latency compilation time perspective.</div><div><br></div><div><br></div><div>[1] <a href="http://llvm.org/docs/WritingAnLLVMPass.html#the-runonfunction-method">http://llvm.org/docs/WritingAnLLVMPass.html#the-runonfunction-method</a></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><div>On Sep 24, 2014, at 10:06 AM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Sep 23, 2014 at 01:57:23PM -0700, Michael Ilseman wrote:<br><blockquote type="cite"><br><blockquote type="cite">On Sep 23, 2014, at 12:48 PM, Tom Stellard <<a href="mailto:thomas.stellard@amd.com">thomas.stellard@amd.com</a>> wrote:<br><br>This pass will expand unsupported div/rem instructions based on<br>the result of the newly added TargetLowering::shouldExpandDivRemInIR().<br>---<br><br>A future commit will enable this pass for the R600 backend and add tests.<br><br>include/llvm/CodeGen/Passes.h | 6 ++<br>include/llvm/InitializePasses.h | 1 +<br>include/llvm/Target/TargetLowering.h | 6 ++<br>lib/CodeGen/CMakeLists.txt | 1 +<br>lib/CodeGen/IntegerDivisionPass.cpp | 124 +++++++++++++++++++++++++++++++++++<br>5 files changed, 138 insertions(+)<br>create mode 100644 lib/CodeGen/IntegerDivisionPass.cpp<br><br>diff --git a/include/llvm/CodeGen/Passes.h b/include/llvm/CodeGen/Passes.h<br>index 31fba89..3fedd5a 100644<br>--- a/include/llvm/CodeGen/Passes.h<br>+++ b/include/llvm/CodeGen/Passes.h<br>@@ -376,6 +376,9 @@ namespace llvm {<br> /// load-linked/store-conditional loops.<br> extern char &AtomicExpandID;<br><br>+ /// Lowers unsupported integer division.<br>+ extern char &IntegerDivisionID;<br>+<br> /// MachineLoopInfo - This pass is a loop analysis pass.<br> extern char &MachineLoopInfoID;<br><br>@@ -598,6 +601,9 @@ namespace llvm {<br><br> /// createJumpInstrTables - This pass creates jump-instruction tables.<br> ModulePass *createJumpInstrTablesPass();<br>+<br>+ /// Lower unsupported integer division<br>+ FunctionPass *createIntegerDivisionPass(const TargetMachine *TM);<br>} // End llvm namespace<br><br>/// This initializer registers TargetMachine constructor, so the pass being<br>diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h<br>index 2964798..99f7ab5 100644<br>--- a/include/llvm/InitializePasses.h<br>+++ b/include/llvm/InitializePasses.h<br>@@ -146,6 +146,7 @@ void initializeInlineCostAnalysisPass(PassRegistry&);<br>void initializeInstCombinerPass(PassRegistry&);<br>void initializeInstCountPass(PassRegistry&);<br>void initializeInstNamerPass(PassRegistry&);<br>+void initializeIntegerDivisionPass(PassRegistry&);<br>void initializeInternalizePassPass(PassRegistry&);<br>void initializeIntervalPartitionPass(PassRegistry&);<br>void initializeJumpInstrTableInfoPass(PassRegistry&);<br>diff --git a/include/llvm/Target/TargetLowering.h b/include/llvm/Target/TargetLowering.h<br>index 0a72225..245df33 100644<br>--- a/include/llvm/Target/TargetLowering.h<br>+++ b/include/llvm/Target/TargetLowering.h<br>@@ -1000,6 +1000,12 @@ public:<br> return false;<br> }<br><br>+ /// Returns true if the instruction should be expanded by the IR-level<br>+ /// IntegerDivision pass.<br>+ virtual bool shouldExpandDivRemInIR(const BinaryOperator &I) const {<br>+ return false;<br>+ }<br>+<br> //===--------------------------------------------------------------------===//<br> // TargetLowering Configuration Methods - These methods should be invoked by<br> // the derived class constructor to configure this object for the target.<br>diff --git a/lib/CodeGen/CMakeLists.txt b/lib/CodeGen/CMakeLists.txt<br>index 0e06ed0..d002f47 100644<br>--- a/lib/CodeGen/CMakeLists.txt<br>+++ b/lib/CodeGen/CMakeLists.txt<br>@@ -25,6 +25,7 @@ add_llvm_library(LLVMCodeGen<br> GlobalMerge.cpp<br> IfConversion.cpp<br> InlineSpiller.cpp<br>+ IntegerDivisionPass.cpp<br> InterferenceCache.cpp<br> IntrinsicLowering.cpp<br> JumpInstrTables.cpp<br>diff --git a/lib/CodeGen/IntegerDivisionPass.cpp b/lib/CodeGen/IntegerDivisionPass.cpp<br>new file mode 100644<br>index 0000000..b198a1f<br>--- /dev/null<br>+++ b/lib/CodeGen/IntegerDivisionPass.cpp<br>@@ -0,0 +1,124 @@<br>+//===-- IntegerDivisionPass.cpp - Expand div/mod instructions -------------===//<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>+/// \file<br>+//===----------------------------------------------------------------------===//<br>+<br>+#include "llvm/CodeGen/Passes.h"<br>+#include "llvm/IR/IRBuilder.h"<br>+#include "llvm/IR/InstVisitor.h"<br>+#include "llvm/Target/TargetLowering.h"<br>+#include "llvm/Target/TargetMachine.h"<br>+#include "llvm/Target/TargetSubtargetInfo.h"<br>+#include "llvm/Transforms/Utils/IntegerDivision.h"<br>+using namespace llvm;<br>+<br>+#define DEBUG_TYPE "integer-division"<br>+<br>+namespace {<br>+<br>+class IntegerDivision : public FunctionPass,<br>+ public InstVisitor<IntegerDivision, bool> {<br>+<br>+ const TargetMachine *TM;<br>+ const TargetLowering *TLI;<br>+<br>+ std::vector<BinaryOperator *> Divs;<br>+ std::vector<BinaryOperator *> Rems;<br>+<br></blockquote><br>As far as I can tell from the patch, these vectors are not used. What is their intent?<br><br></blockquote><br>These are leftover from a previous version of the patch. Here is an updated<br>patch with them removed.<br><br>-Tom<br><span><0001-CodeGen-Add-IntegerDivisionPass.patch></span></div></blockquote></div><br></body></html>