<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On May 5, 2010, at 1:34 PM, Nathan Jeffords wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">After working with Aaron Gray for the last few days, I believe we have a version of my COFF object file support that can be merged into LLVM trunk. It is able to generate usable object files for ANSI C programs, but lacks necessary support any other non "C" linkage types.<br></blockquote></div><br><div>Hi Nathan,</div><div><br></div><div>Thanks for working on this, I haven't looked at the algorithmic portions of this, but here are a few comments on style. A high level comment is that I think you should start with getting MCSectionCOFF squared away before you do much of anything else. Also, consider that you are effectively implementing a COFF assembler here.</div><div><br></div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+++ tools/llc/llc.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">@@ -119,7 +119,8 @@</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> return outputFilename;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> }</div><p style="margin: 0.0px 0.0px 0.0px 0.0px; font: 12.0px Inconsolata; min-height: 13.0px"> <br class="webkit-block-placeholder"></p><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">-static formatted_raw_ostream *GetOutputStream(const char *TargetName, </div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+static formatted_raw_ostream *GetOutputStream(const char *TargetName,</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+<span class="Apple-tab-span" style="white-space:pre"> </span> Triple::OSType OS,</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> const char *ProgName) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> if (OutputFilename != "") {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> if (OutputFilename == "-")</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">@@ -166,7 +167,10 @@</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> OutputFilename += ".s";</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> break;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> case TargetMachine::CGFT_ObjectFile:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">- OutputFilename += ".o";</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ if (OS == Triple::Win32)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ OutputFilename += ".obj";</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+<span class="Apple-tab-span" style="white-space:pre"> </span>else</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ OutputFilename += ".o";</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> Binary = true;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> break;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> case TargetMachine::CGFT_Null:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">@@ -284,7 +288,7 @@</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> TargetMachine &Target = *target.get();</div><p style="margin: 0.0px 0.0px 0.0px 0.0px; font: 12.0px Inconsolata; min-height: 13.0px"> <br class="webkit-block-placeholder"></p><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> // Figure out where we are going to send the output...</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">- formatted_raw_ostream *Out = GetOutputStream(TheTarget->getName(), argv[0]);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ formatted_raw_ostream *Out = GetOutputStream(TheTarget->getName(), TheTriple.getOS (), argv[0]);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> if (Out == 0) return 1;</div><p style="margin: 0.0px 0.0px 0.0px 0.0px; font: 12.0px Inconsolata; min-height: 13.0px"> <br class="webkit-block-placeholder"></p><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> CodeGenOpt::Level OLvl = CodeGenOpt::Default;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">The first hunk is formatted wrong. The second hunk looks fine except for the tabs. In any case, this part of the patch can go in before everything else, please send it as a stand-alone patch.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">Please follow the formatting of the rest of the llvm codebase, instead of:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+namespace llvm</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+{</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ MCObjectWriter *createWinCOFFObjectWriter(raw_ostream & OS) {</div><div><br></div><div>Use:</div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+namespace llvm {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ MCObjectWriter *createWinCOFFObjectWriter(raw_ostream &OS) {</div><div><br></div></div><div>fixing the { and &</div><div><br></div><div>Likewise, fit the code in 80 columns, don't use tabs, put the { on the same line as the if.</div><div><br></div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+#include <stdio.h></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+#ifndef NDEBUG</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+#define _dbg(x) x</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+#else</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+#define _dbg(x)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+#endif</div><div><br></div><div>Please use the DEBUG macro instead of rolling your own.</div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">--- lib/MC/WinCoffStreamer.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+++ lib/MC/WinCoffStreamer.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">@@ -0,0 +1,500 @@</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+//===-- llvm/MC/WinCOFFStreamer.cpp -------------------------*- C++ -*-===//</div><div><br></div><div>Please make sure the case of hte filename (Coff) matches the case in the files. I think that COFF is correct (since it is an acronym) and consistent with the rest of the codebase.</div></div></div></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">@@ -57,7 +57,7 @@</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> bool X86AsmPrinter::runOnMachineFunction(MachineFunction &MF) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> SetupMachineFunction(MF);</div><p style="margin: 0.0px 0.0px 0.0px 0.0px; font: 12.0px Inconsolata; min-height: 13.0px"> <br class="webkit-block-placeholder"></p><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">- if (Subtarget->isTargetCOFF()) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ if (Subtarget->isTargetCOFF() && OutStreamer.hasRawTextSupport ()) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> const Function *F = MF.getFunction();</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> OutStreamer.EmitRawText("\t.def\t " + Twine(CurrentFnSym->getName()) +</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> ";\t.scl\t" +</div><div><br></div><div>This is *definitely* incorrect. You need to add MCStreamer APIs for the COFF directives that don't have them.</div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+++ lib/CodeGen/TargetLoweringObjectFileImpl.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">@@ -822,6 +822,7 @@</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> TargetLoweringObjectFile::Initialize(Ctx, TM);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> TextSection = getCOFFSection("\t.text", true, SectionKind::getText());</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "> DataSection = getCOFFSection("\t.data", true, SectionKind::getDataRel());</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">+ BSSSection = getCOFFSection(".bss", true, SectionKind::getBSS());</div><div><br></div><div>Why is this needed? You shouldn't have to change the lowering at all.</div><div><br></div><div>-Chris</div><div><br></div></div><div><br></div><div><br></div></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; "><br></div></div></body></html>