<br><br><div class="gmail_quote">2008/7/10 Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>>:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style=""><br><div><div class="Ih2E3d"><div>On Jul 9, 2008, at 5:14 AM, Csaba Hruska wrote:</div><br><blockquote type="cite">Hi!<br>Here are the cutted patch files of indepenent changes:</blockquote><div><br></div></div>
<div>Great!</div><div class="Ih2E3d"><br><blockquote type="cite"><br>TranslationUnit.patch - adds support (de)serialization (from)to (in memory) buffer.</blockquote><div><br></div></div><div>Ted, can you look at this?</div>
<div class="Ih2E3d"><br><blockquote type="cite"><br>Preprocessor.patch - adds a new method (getPredefines) needed for passing predefines list to clangserver.</blockquote><div><br></div></div><div>Looks good to me, applied!</div>
<div class="Ih2E3d"><br><blockquote type="cite"><br> ASTConsumers.patch - adds functions what supports capruing ASTConsumer's output.</blockquote><div><br></div></div><div><div style="margin: 0px;">-ASTConsumer *clang::CreateASTDumper() { return new ASTDumper(); }</div>
<div style="margin: 0px;">+ASTConsumer *clang::CreateASTDumper(std::ostream* OS) { return new ASTDumper(OS); }</div><p style="margin: 0px; font-family: Monaco; font-style: normal; font-variant: normal; font-weight: normal; font-size: 10px; line-height: normal; font-size-adjust: none; font-stretch: normal; min-height: 14px;">
<br></p><div><font size="2" face="Monaco"><span style="font-size: 10px;">Please stay in 80 cols.</span></font></div><div><font size="2" face="Monaco"><span style="font-size: 10px;"><br></span></font></div><div><font size="2" face="Monaco"><span style="font-size: 10px;"><div style="margin: 0px;">
ASTConsumer* CreateHTMLPrinter(const std::string &OutFile, Diagnostic &D,</div><div style="margin: 0px;"> Preprocessor *PP, PreprocessorFactory* PPF);</div><p style="margin: 0px; font-family: Monaco; font-style: normal; font-variant: normal; font-weight: normal; font-size: 10px; line-height: normal; font-size-adjust: none; font-stretch: normal; min-height: 14px;">
<br></p><div style="margin: 0px;">+ASTConsumer* CreateHTMLPrinter(std::ostream &OutStream, Diagnostic &D,</div><div style="margin: 0px;">+ Preprocessor *PP, PreprocessorFactory* PPF);</div>
<div><br></div><div>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?</div>
<div><br></div><div><br></div></span></font></div></div><div class="Ih2E3d"><div><blockquote type="cite">I've also attached a skeleton of distclang documentation page (no info yet, it will be added after the initial commit).<br>
</blockquote><br></div></div><div>Applied.</div><div class="Ih2E3d"><div><br></div><blockquote type="cite"><br>I haven't attached clang.cpp, clang.h NetSession.cpp, NetSession.h patch because they place in source structure is unknown.</blockquote>
<blockquote type="cite">We have to refactor the Driver directory content first.</blockquote><div><br></div></div>Sounds good. Thank you for taking this in steps, it makes it far easier to review.</div><div><br></div><div>
<div class="Ih2E3d"><br><blockquote type="cite"><br> - platform independent thread and network support. Should these added to llvm/Support or System library ?</blockquote><div><br></div></div><div>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.</div>
<div class="Ih2E3d"><br><blockquote type="cite">We also have to decide if we want to be distcc compatible or not. And what's the benefit of distcc compatibility?<br></blockquote></div></div><br><div>I don't really have an opinion or an answer here. :) Are there benefits? anyone have an opinion?</div>
<div><br></div><div><br></div><div><div class="Ih2E3d"><div><blockquote type="cite"><span> - 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)</span></blockquote>
</div></div><div><div><br></div><div>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.</div>
<div><br></div><div>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:</div>
<div><br></div><div>class outstream {</div><div><div style="margin: 0px;"><span style="color: rgb(170, 13, 145);"> </span> <span style="color: rgb(170, 13, 145);">char</span> *OutBufStart, *OutBufEnd, *OutBufCur;</div><div style="margin: 0px;">
outstream(const outstream&); // can't be copied.</div><div style="margin: 0px;"> void operator=(const outstream&); // or assigned.</div><div style="margin: 0px;">protected:</div><div style="margin: 0px;"> <span style="font-family: Helvetica; font-size: 12px;">outstream() {} // may only be subclassed</span></div>
<div style="margin: 0px;"><font size="3" face="Helvetica"><span style="font-size: 12px;"> virtual void FlushBuffer() = 0;</span></font></div><div>public:</div><div> virtual ~outstream();</div></div><div> </div><div>
void OutputChar(char c) { ...} </div><div> void OutputString(<span style="color: rgb(170, 13, 145); font-family: Monaco; font-size: 10px;">const<span style="color: rgb(0, 0, 0);"> </span>char<span style="color: rgb(0, 0, 0);"> *Ptr, </span>unsigned<span style="color: rgb(0, 0, 0);"> Size) { ... }</span></span></div>
<div> void Close();</div><div><br></div><div> static outstream* CreateFile(...);</div><div> static outstream* CreateOStream(std::ostream &O);</div><div> ...</div><div>};</div><div><br></div><div>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).</div>
<div><br></div><div>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).</div>
</div></div></div></blockquote><div>This can solve my problem. And if we make the ASTConsumer library they should use the outstream class to write out they output (This will remove duplications from ASTConsumer.h functions).<br>
There is one more requirement if we follow this. Most of ASTConsumers have one output stream or file, but there is StaticAnalysis what is make a couple of HTML files and because distcc static analysis is a big speedup, we must support this astconsumer, so we have to capture its output somehow. Maybe with some outstream manager interface class, one implementation for normal clang to support files and stdout and one implementation for clangserver for netstreams. The outstream class interface is ok for this except we cannot overload CreateFile and CreateOStream.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div><div><div></div><div><br></div></div></div><font color="#888888"><div>
-Chris</div></font></div></blockquote></div><br><br>Csaba<br>