[PATCH] MC: Implement COFF .linkonce directive

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Fri Jul 5 05:45:46 PDT 2013



================
Comment at: include/llvm/MC/MCSectionCOFF.h:39
@@ -35,1 +38,3 @@
+    /// associative Selection (IMAGE_COMDAT_SELECT_ASSOCIATIVE).
+    mutable const MCSectionCOFF *Assoc;
 
----------------
Why these fields need to be mutable? The equivalent ones in MCSectionELF are not.

================
Comment at: docs/Extensions.rst:42-43
@@ -39,1 +41,4 @@
     .long $unwind$fun at imgrel
+
+
+``.linkonce`` Directive
----------------
This is the same as the gnu assembler, no? Do we consider it an extension because it is not what the native tools do?

================
Comment at: include/llvm/MC/MCSectionCOFF.h:71
@@ -61,1 +70,3 @@
+
+    void setSelection(int Selection, const MCSectionCOFF *Assoc = 0) const;
 
----------------
If you remove this const you can also remove the mutable, no?

================
Comment at: lib/MC/MCParser/COFFAsmParser.cpp:458
@@ +457,3 @@
+
+  Current->setSelection(Type, Assoc);
+
----------------
What is the correct behavior if  .linkonce is specified twice, should we produce an error?

================
Comment at: lib/MC/MCSectionCOFF.cpp:38
@@ +37,3 @@
+  this->Assoc = Assoc;
+  if (Selection != 0)
+    Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
----------------
We never pass 0, it would be better to assert that.


http://llvm-reviews.chandlerc.com/D1100



More information about the llvm-commits mailing list