<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 5:49 PM, Nathan Jeffords wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br><br><div class="gmail_quote">On Wed, May 5, 2010 at 5:22 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><div>... </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><div class="im"></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+++ tools/llc/llc.cpp<span style="white-space:pre">     </span>(working copy)</div></div>
</div></blockquote><div>...  </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div style="word-wrap:break-word"><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
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">
<br></div></div></div></blockquote><div><br></div><div>OK, fixed the formatting problems and have attached the patch to llc.cpp alone</div></div></blockquote><div><br></div><div>Thanks, applied in r103150!  llvm-mc -filetype=obj probably needs a similar patch.</div><div><br></div><div><br></div><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div style="word-wrap:break-word"><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
Please follow the formatting of the rest of the llvm codebase, instead of:</div></div></div></blockquote><div>...</div><div> </div><div>I'm trying real hard :) </div></div></blockquote><div><br></div>:)</div><div><br><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div style="word-wrap:break-word"><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><div><br><br></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+#include <stdio.h></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+#ifndef NDEBUG</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+#define _dbg(x) x</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+#else</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+#define _dbg(x)</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+#endif</div><div><br></div><div>Please use the DEBUG macro instead of rolling your own.</div></div></div></div></div></blockquote>
<div><br></div><div>this macro allows me to optionally declare stuff where the DEBUG macro will not...</div></div></blockquote><div><br></div><div>we prefer to declare variables unconditionally.  It makes the code less fragile, though you do have to watch out of "unused variable warnings.  A common idiom would be.</div><div><br></div><div>int whatever = 0;  (void)whatever;</div><div>DEBUG(whatever = foo());</div><div>...</div><div><br></div><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div style="word-wrap:break-word"><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">@@ -57,7 +57,7 @@</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"> bool X86AsmPrinter::runOnMachineFunction(MachineFunction &MF) {</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
   SetupMachineFunction(MF);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; min-height: 13px; "> <br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">-  if (Subtarget->isTargetCOFF()) {</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+  if (Subtarget->isTargetCOFF() && OutStreamer.hasRawTextSupport ()) {</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
     const Function *F = MF.getFunction();</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">     OutStreamer.EmitRawText("\t.def\t " + Twine(CurrentFnSym->getName()) +</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">                             ";\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></div></div></blockquote><div><br></div><div>So I should turn this into EmitCOFFFunctionDefinition or something of that nature?</div></div></blockquote><div><br></div><div>Yes probably, I don't know what the .def and .scl directives "do" :)</div><div><br></div><div>Also, w.r.t. section handling stuff, there is this fixme in the asmprinter:</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; ">    } <span style="color: #b50da1">else</span> <span style="color: #b50da1">if</span> (<span style="color: #b50da1">const</span> <span style="color: #b50da1">char</span> *LinkOnce = MAI->getLinkOnceDirective()) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(20, 135, 11); "><span style="color: #000000">      </span>// .globl _foo</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">      OutStreamer.EmitSymbolAttribute(GVSym, MCSA_Global);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(20, 135, 11); "><span style="color: #000000">      </span>// FIXME: linkonce should be a section attribute, handled by COFF Section</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(20, 135, 11); "><span style="color: #000000">      </span>// assignment.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(58, 0, 252); "><span style="color: #000000">      </span><span style="color: #14870b">// <a href="http://sourceware.org/binutils/docs-2.20/as/Linkonce.html#Linkonce"><span style="color: #3a00fc">http://sourceware.org/binutils/docs-2.20/as/Linkonce.html#Linkonce</span></a></span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(20, 135, 11); "><span style="color: #000000">      </span>// .linkonce discard</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(20, 135, 11); "><span style="color: #000000">      </span>// FIXME: It would be nice to use .linkonce samesize for non-common</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; color: rgb(20, 135, 11); "><span style="color: #000000">      </span>// globals.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">      OutStreamer.EmitRawText(StringRef(LinkOnce));</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 12px/normal Inconsolata; ">    } <span style="color: #b50da1">else</span> {</div><div><font class="Apple-style-span" face="Inconsolata"><br></font></div><div><font class="Apple-style-span" face="Inconsolata">Basically, it seems like the MCSectionCOFF implementation should get the linkonce bit.  I'm not an expert on COFF, but I think that's the right place for it.  ".linkonce" will also have to be added as a new MCStreamer api, which would set the bit.</font></div><div><font class="Apple-style-span" face="Inconsolata"><br></font></div></div></div><div>Thanks Nathan!</div><div><br></div><div>-Chris</div></body></html>