[llvm-commits] Win32 COFF Support

[Chris Lattner]

On May 5, 2010, at 5:49 PM, Nathan Jeffords wrote:

> 
> 
> On Wed, May 5, 2010 at 5:22 PM, Chris Lattner <clattner at apple.com> wrote:
> ... 
> +++ tools/llc/llc.cpp	(working copy)
> ...  
> 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.
> 
> 
> OK, fixed the formatting problems and have attached the patch to llc.cpp alone

Thanks, applied in r103150!  llvm-mc -filetype=obj probably needs a similar patch.


> 
> Please follow the formatting of the rest of the llvm codebase, instead of:
> ...
>  
> I'm trying real hard :) 

:)

> 
> 
> +
> +#include <stdio.h>
> +
> +#ifndef NDEBUG
> +#define _dbg(x) x
> +#else
> +#define _dbg(x)
> +#endif
> 
> Please use the DEBUG macro instead of rolling your own.
> 
> this macro allows me to optionally declare stuff where the DEBUG macro will not...

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.

int whatever = 0;  (void)whatever;
DEBUG(whatever = foo());
...

> @@ -57,7 +57,7 @@
>  bool X86AsmPrinter::runOnMachineFunction(MachineFunction &MF) {
>    SetupMachineFunction(MF);
>  
> -  if (Subtarget->isTargetCOFF()) {
> +  if (Subtarget->isTargetCOFF() && OutStreamer.hasRawTextSupport ()) {
>      const Function *F = MF.getFunction();
>      OutStreamer.EmitRawText("\t.def\t " + Twine(CurrentFnSym->getName()) +
>                              ";\t.scl\t" +
> 
> This is *definitely* incorrect.  You need to add MCStreamer APIs for the COFF directives that don't have them.
> 
> So I should turn this into EmitCOFFFunctionDefinition or something of that nature?

Yes probably, I don't know what the .def and .scl directives "do" :)

Also, w.r.t. section handling stuff, there is this fixme in the asmprinter:

    } else if (const char *LinkOnce = MAI->getLinkOnceDirective()) {
      // .globl _foo
      OutStreamer.EmitSymbolAttribute(GVSym, MCSA_Global);
      // FIXME: linkonce should be a section attribute, handled by COFF Section
      // assignment.
      // http://sourceware.org/binutils/docs-2.20/as/Linkonce.html#Linkonce
      // .linkonce discard
      // FIXME: It would be nice to use .linkonce samesize for non-common
      // globals.
      OutStreamer.EmitRawText(StringRef(LinkOnce));
    } else {

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.

Thanks Nathan!

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100505/af0933d1/attachment.html>