On Mar 12, 2010, at 8:10 AM, David Greene wrote:> On Friday 12 March 2010 08:13:05 Kalle Raiskila wrote:
>> David Greene wrote:
>>> Here's a rework using PassManager as Chris suggested.
Comments?
>>
>> Tried this second patch with the svn version 97812 (the one the patch
is
>> made against), but it doesn't compile:
>> "llvm/include/llvm/Pass.h:127: Error: expected unqualified-id
before "&"
>> token"
>> Seems raw_ostream is forward declared but not defined (adding a missing
>> #include fixed that).
>
> It shouldn't need to be defined. Perhaps std::string is the issue,
though I
> don't know why I don't see the problem. Probably because
> MachineFunctionPrinterPass.h includes <string> somewhere along
> the line.
This is much better than the first iteration but still has many issues. There
is no need to keep cc'ing cfe-dev on these patches which have nothing to do
with clang.
Please rename the getPrinterPass method to createPrinterPass to indicate that
the result returns a new pass, not returning an existing one. Please make it
take a StringRef, not an std::string and eliminate the <string> #include
from Pass.h.
MachineFunctionPrinterPass.h doesn't need to #include MachineFunctionPass.h,
just use a forward declaration. Also, please move the prototype of
createMachineFunctionPrinterPass into include/llvm/CodeGen/Passes.h, eliminating
the need for the header in the first place.
This hunk looks unrelated, please remove it:
+++ include/llvm/PassManager.h (working copy)
@@ -18,6 +18,7 @@
#define LLVM_PASSMANAGER_H
#include "llvm/Pass.h"
+#include "llvm/Support/PassNameParser.h"
namespace llvm {
+Pass *CallGraphSCCPass::getPrinterPass(raw_ostream &O,
+ const std::string &Banner) const {
+ return createPrintModulePass(Banner, &O);
+}
This isn't correct at all, this should return a CGSCCPass as a printer.
Adding a Module pass will change the ordering of passes in the passmanager,
which will cause your option to completely change behavior of the optimization
sequence.
+Pass *LoopPass::getPrinterPass(raw_ostream &O,
+ const std::string &Banner) const {
+ return createPrintFunctionPass(Banner, &O);
+}
likewise. The printer needs to be a loop pass.
+++ lib/CodeGen/MachineFunctionPrinterPass.cpp (revision 0)
@@ -0,0 +1,51 @@
+//===-- MachineFunction.cpp
-----------------------------------------------===//
Please update the comment.
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineFunctionPrinterPass.h"
+#include "llvm/Support/raw_ostream.h"
Include the file's interface first. This should be CodeGen/Passes.h after
the change requested above.
+namespace llvm {
+struct MachineFunctionPrinterPass : public MachineFunctionPass {
This can be in an anonymous namespace, please add a doxygen comment.
+++ lib/Target/X86/X86ISelDAGToDAG.cpp (working copy)
@@ -1548,6 +1548,107 @@
+ case ISD::STORE: {
+ // Handle unaligned non-temporal stores.
+ StoreSDNode *ST = cast<StoreSDNode>(Node);
+ DebugLoc dl = Node->getDebugLoc();
+ EVT VT = ST->getMemoryVT();
+ if (VT.isVector() &&
+ VT.isFloatingPoint() &&
+ VT.getSizeInBits() == 128 &&
+ ST->getAlignment() < 16) {
+ // Unaligned stores
This is completely unrelated to your patch.
+++ lib/VMCore/PassManager.cpp (working copy)
#include "llvm/PassManagers.h"
+#include "llvm/Assembly/PrintModulePass.h"
#include "llvm/Assembly/Writer.h"
#include "llvm/Support/CommandLine.h"
You don't need this #include.
+// Register a pass and add options to dump the IR before and after it
+// is run
+typedef llvm::cl::list<const llvm::PassInfo *, bool, PassNameParser>
+PassOptionList;
This comment is out of date.
+// Print IR out before/after specified passes
+PassOptionList
+PrintBefore("print-before",
+ llvm::cl::desc("Print IR before specified passes"));
Please properly punctuate your comment.
+namespace {
+/// This is a utility to check whether a pass shoulkd have IR dumped
+/// before it.
+bool PrintBeforePass(Pass *P) {
Please just mark stand-alone functions "static" don't put them in
anonymous namespaces. Typo in the comment. Please rename this to
"ShouldPrintBeforePass", "PrintBeforePass" implies that it
does printing.
+ bool PrintBeforeThis = PrintBeforeAll;
+ if (!PrintBeforeThis)
don't nest the entire function, use an early return like this:
if (PrintBeforeAll) return true;
+ for (unsigned i = 0; i < PrintBefore.size(); ++i) {
Don't evaluate PrintBefore.size() every time through the loop.
+ if (PassInf && P->getPassInfo())
+ if (PassInf->getPassArgument() =+
P->getPassInfo()->getPassArgument()) {
+ PrintBeforeThis = true;
+ break;
+ }
+ }
+ return PrintBeforeThis;
+}
Instead of using break with a variable, just "return true;" out of the
loop.
Please merge and factor PrintBeforePass/PrintAfterPass instead of copying and
pasting it.
+++ lib/VMCore/PrintModulePass.cpp (working copy)
@@ -20,24 +20,25 @@
#include "llvm/Support/raw_ostream.h"
using namespace llvm;
-namespace {
+namespace llvm {
Why did you change this?
Please make these modification and verify that regression tests pass with *only*
this patch applied to a mainline tree.
-Chris