[cfe-dev] final distributed clang patch

[Chris Lattner]

On Jul 9, 2008, at 5:14 AM, Csaba Hruska wrote:

> Hi!
> Here are the cutted patch files of indepenent changes:

Great!

>
> TranslationUnit.patch - adds support (de)serialization (from)to (in  
> memory) buffer.

Ted, can you look at this?

>
> Preprocessor.patch - adds a new method (getPredefines) needed for  
> passing predefines list to clangserver.

Looks good to me, applied!

>
> ASTConsumers.patch - adds functions what supports capruing  
> ASTConsumer's output.

-ASTConsumer *clang::CreateASTDumper() { return new ASTDumper(); }
+ASTConsumer *clang::CreateASTDumper(std::ostream* OS) { return new  
ASTDumper(OS); }

Please stay in 80 cols.

  ASTConsumer* CreateHTMLPrinter(const std::string &OutFile,  
Diagnostic &D,
                                 Preprocessor *PP,  
PreprocessorFactory* PPF);

+ASTConsumer* CreateHTMLPrinter(std::ostream &OutStream, Diagnostic &D,
+                               Preprocessor *PP, PreprocessorFactory*  
PPF);

It's unclear to me that duplicating each of these is really the right  
approach.  Would it work to make the *only* entry points take an  
ostream, and then just have the caller create the ofstream etc and  
pass that in as the OutStream?


> I've also attached a skeleton of distclang documentation page (no  
> info yet, it will be added after the initial commit).

Applied.

>
> I haven't attached clang.cpp, clang.h NetSession.cpp, NetSession.h  
> patch because they place in source structure is unknown.
> We have to refactor the Driver directory content first.

Sounds good.  Thank you for taking this in steps, it makes it far  
easier to review.


>
>  - platform independent thread and network support. Should these  
> added to llvm/Support or System library ?

The general rule is that really low level things that are highly  
system specific should go in libsystem.  libsupport can contain target  
independent abstractions that may optionally be built on libsystem  
components.

> We also have to decide if we want to be distcc compatible or not.  
> And what's the benefit of distcc compatibility?

I don't really have an opinion or an answer here. :)  Are there  
benefits? anyone have an opinion?


>  - capture output of PrintPreprocessedOutput.cpp. Ted: openfd is not  
> acceptable, because we have to know the output size before we send  
> it. (required by net packet header)


Lets talk about this one specifically.  The "Simple buffered I/O" code  
in PrintPreprocessedOutput.cpp was written with extensive profiling,  
tuning and other tweaking and it is quite effective, though very ugly,  
code.  The basic algorithm is that it buffers up chunks of 64K, then  
writes them out to disk with open/write/close.  The API for it is very  
simple, and is basically built around not copying strings unnecessarily.

This is pretty simple stuff and would be very useful elsewhere in LLVM  
(e.g. the .s printer, the bc writer, ...).  It has been on my todo  
list for a long time, and seems required for the distcc project, to  
refactor this out to be a more general interface.  I think something  
like this would be a great interface:

class outstream {
   char *OutBufStart, *OutBufEnd, *OutBufCur;
   outstream(const outstream&); // can't be copied.
   void operator=(const outstream&); // or assigned.
protected:
   outstream() {}  // may only be subclassed
     virtual void FlushBuffer() = 0;
public:
   virtual ~outstream();

   void OutputChar(char c) { ...}
   void OutputString(const char *Ptr, unsigned Size) { ... }
   void Close();

   static outstream* CreateFile(...);
   static outstream* CreateOStream(std::ostream &O);
   ...
};

The basic idea here is that all the buffering of simple things (e.g.  
strings and chars) is inline and trivial, and the flushing happens via  
a virtual method that does the write or whatever else is needed.  This  
is a very simple and useful API that is mostly target-independent (and  
should thus live in libsupport) but does rely on a few system specific  
things (which could live in libsystem).

Is this something you could take on?  It should be pretty straight- 
forward.  Once it exists, switching the -E printer over to it should  
be easy, and this will make it easy to get it to output to an  
std::ostream or whatever else is desired (you could even make an  
outstream for a socket or whatever, to avoid the extra std::ostream  
overhead).

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080709/21849c92/attachment.html>