[llvm-commits] Win32 COFF Support

[Nathan Jeffords]

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



>
> 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...

...

> 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.
>
>
OK


> @@ -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?


>
> +++ lib/CodeGen/TargetLoweringObjectFileImpl.cpp (working copy)
> @@ -822,6 +822,7 @@
>    TargetLoweringObjectFile::Initialize(Ctx, TM);
>    TextSection = getCOFFSection("\t.text", true, SectionKind::getText());
>    DataSection = getCOFFSection("\t.data", true,
> SectionKind::getDataRel());
> +  BSSSection = getCOFFSection(".bss", true, SectionKind::getBSS());
>
> Why is this needed?  You shouldn't have to change the lowering at all.
>

I see that now after the conversation about uninitialized global variables
and common symbols, I will fix that.

- Nathan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100505/1ec85cd4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llc.patch
Type: application/octet-stream
Size: 1302 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100505/1ec85cd4/attachment.obj>