Thanks for the feedback.<br><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">The line for commit-worthy is a round of reviews and some tests<br>

showing that basic functionality works.<br></blockquote><div><br></div><div>Regarding tests, I would like to have some form of automated regression testing, but am not sure how to go about doing so. My current testing involves running through my compilers regression test (compiles links and executes a number of test sources) and running a couple of .ll files and manually inspecting the results of Microsoft's dumpbin utility.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Minor comments:<br>
1. Is it possible to allocate extra memory with coff::symbol instead<br>
of using std::string and std::vector as class members? The extra<br>
allocations can add up.<br></blockquote><div><br></div><div>Yes, I believe ShortString & ShortVector are the appropriate alternative for these types of cases.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<br>
2. For the following bit:<br>
+    reinterpret_cast <uint32_t &> (Section->Symbol->Aux [0]) =<br>
Section->Header.SizeOfRawData;<br>
+    reinterpret_cast <uint16_t &> (Section->Symbol->Aux [4]) =<br>
Section->Header.NumberOfRelocations;<br>
+    reinterpret_cast <uint16_t &> (Section->Symbol->Aux [6]) =<br>
Section->Header.PointerToLineNumbers;<br>
<br>
This isn't endian-safe; I'd suggest expanding it to 8 assignments.<br>
Same issues with the write_le functions.  We need endian safety not<br>
only so that cross-compiling, for example, from PPC Linux to MingW<br>
works, but also so that tests work cross-platform.<br></blockquote><div><br></div><div>OK, I had put the write_le functions so that I could address this later. I should have used them, or something similar for the auxiliary symbols. I will rework the aux symbol stuff & the write_le functions to be cross platform safe.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
3. For the following bit:<br>
+    typedef std::list <symbol*>  symbols;<br>
+    typedef std::list <section*> sections;<br>
<br>
Avoid std::list; see <a href="http://llvm.org//docs/ProgrammersManual.html#dss_list" target="_blank">http://llvm.org//docs/ProgrammersManual.html#dss_list</a> .<br></blockquote><div><br></div><div>Good catch, I would normally not do that. I must have had a list of objects, then changed it to pointers to objects and didn't rethink what type of container I was using.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
4. For the following bit:<br>
+    typedef StringMap <coff::symbol *>  name_symbol_map;<br>
+    typedef StringMap <coff::section *> name_section_map;<br>
<br>
Unused typedefs?<br></blockquote><div><br></div><div>Left over from previous version of the implementation, will remove.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<br>
5. For the following bit (and a few others in the same function):<br>
+    for (i = Sections.begin(), j = Asm.begin();<br>
+         (i != Sections.end()) && (j != Asm.end());<br>
+         i++, j++) {<br>
<br>
<a href="http://llvm.org//docs/CodingStandards.html#ll_end" target="_blank">http://llvm.org//docs/CodingStandards.html#ll_end</a><br>
<br></blockquote><div><br></div><div>I will recode those statements.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
6.<br>
+namespace llvm { MCObjectWriter * createWinCOFFObjectWriter<br>
(raw_ostream & OS); }<br>
<br>
If you need a function to be visible across files, use a header;<br>
shortcuts like this make the code more difficult to read.<br>
<font color="#888888"><br></font></blockquote><div> </div><div>On this one, it didn't seem appropriate to include WinCOFFObjectWriters header into X86AsmBackend, it also seems to be overkill to add a header to contain a single function. I would like some more feedback on what the sanctioned course of action should be here. </div>
<div></div></div><br><div>- Nathan</div>