[llvm-commits] Win32 COFF Support

[Chris Lattner]

On May 5, 2010, at 1:34 PM, Nathan Jeffords wrote:

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

Hi Nathan,

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.


+++ tools/llc/llc.cpp	(working copy)
@@ -119,7 +119,8 @@
   return outputFilename;
 }
 
-static formatted_raw_ostream *GetOutputStream(const char *TargetName, 
+static formatted_raw_ostream *GetOutputStream(const char *TargetName,
+											  Triple::OSType OS,
                                               const char *ProgName) {
   if (OutputFilename != "") {
     if (OutputFilename == "-")
@@ -166,7 +167,10 @@
       OutputFilename += ".s";
     break;
   case TargetMachine::CGFT_ObjectFile:
-    OutputFilename += ".o";
+    if (OS == Triple::Win32)
+      OutputFilename += ".obj";
+	else
+      OutputFilename += ".o";
     Binary = true;
     break;
   case TargetMachine::CGFT_Null:
@@ -284,7 +288,7 @@
   TargetMachine &Target = *target.get();
 
   // Figure out where we are going to send the output...
-  formatted_raw_ostream *Out = GetOutputStream(TheTarget->getName(), argv[0]);
+  formatted_raw_ostream *Out = GetOutputStream(TheTarget->getName(), TheTriple.getOS (), argv[0]);
   if (Out == 0) return 1;
 
   CodeGenOpt::Level OLvl = CodeGenOpt::Default;

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.


Please follow the formatting of the rest of the llvm codebase, instead of:

+namespace llvm
+{
+  MCObjectWriter *createWinCOFFObjectWriter(raw_ostream & OS) {

Use:

+namespace llvm {
+  MCObjectWriter *createWinCOFFObjectWriter(raw_ostream &OS) {

fixing the { and &

Likewise, fit the code in 80 columns, don't use tabs, put the { on the same line as the if.


+
+#include <stdio.h>
+
+#ifndef NDEBUG
+#define _dbg(x) x
+#else
+#define _dbg(x)
+#endif

Please use the DEBUG macro instead of rolling your own.

--- lib/MC/WinCoffStreamer.cpp	(revision 0)
+++ lib/MC/WinCoffStreamer.cpp	(revision 0)
@@ -0,0 +1,500 @@
+//===-- llvm/MC/WinCOFFStreamer.cpp -------------------------*- C++ -*-===//

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.


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

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

-Chris





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