[PATCH] Alternative way of representing comdats

David Majnemer david.majnemer at gmail.com
Thu Jun 26 16:11:44 PDT 2014


I've added a test for ELF.

================
Comment at: docs/LangRef.rst:746
@@ +745,3 @@
+    The linker may choose any COMDAT key, the choice is arbitrary.
+``exactmatch``
+    The linker may choose any COMDAT key but the sections must contain the
----------------
Rafael Ávila de Espíndola wrote:
> The sections or the comdat symbols btw? What does link.exe do?
Sections, COFF doesn't know how big a symbol is.

================
Comment at: docs/LangRef.rst:751
@@ +750,3 @@
+    The linker will choose the section containing the largest COMDAT key.
+``newest``
+    The linker will choose the section containing the newest COMDAT key.
----------------
Rafael Ávila de Espíndola wrote:
> This is undocumented and it seems unused. How about removing it for now?
Done.

================
Comment at: docs/LangRef.rst:756
@@ +755,3 @@
+``samesize``
+    The linker may choose any COMDAT key but the sections must contain the
+    same amount of data.
----------------
Rafael Ávila de Espíndola wrote:
> The sections or the comdat symbol must be the same size?
Sections, COFF doesn't know how big a symbol is.

================
Comment at: docs/LangRef.rst:763
@@ +762,3 @@
+Here is an example of a COMDAT group where a function will only be selected if
+the COMDAT key's section is the largest:
+
----------------
Rafael Ávila de Espíndola wrote:
> "selected if the COMDAT's key is the largest."
> 
> i.e., we have to drop the "section", no?
> 
Your understanding is correct.

================
Comment at: docs/LangRef.rst:785
@@ -719,3 +784,3 @@
 .. _namedmetadatastructure:
 
 Named Metadata
----------------
Rafael Ávila de Espíndola wrote:
> This is still missing:
> 
> What I meant about IR sections being split into multiple object sections is that in, for example,
> 
>   $foo = comdat any
>   $bar = comdat any
>   @g1 = global i32 42, section "sec", comdat $foo
>   @g2 = global i32 42, section "sec", comdat $bar
> 
> there is only one section "sec", but in the object file we end up with two sections named sec:
> 
>   [ 6] sec               PROGBITS        0000000000000000 000050 000004 00 WAG  0   0  4
>   [ 7] sec               PROGBITS        0000000000000000 000054 000004 00 WAG  0   0  4
> 
Done.

================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:796
@@ +795,3 @@
+  if (!ComdatGV)
+    report_fatal_error("Associative COMDAT symbol '" + ComdatGVName +
+                       "' does not exist.");
----------------
Rafael Ávila de Espíndola wrote:
> needs test
Done.

================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:800
@@ +799,3 @@
+  if (ComdatGV->getComdat() != C)
+    report_fatal_error("Associative COMDAT symbol '" + ComdatGVName +
+                       "' is not a key for it's COMDAT.");
----------------
Rafael Ávila de Espíndola wrote:
> needs test.
Done.

================
Comment at: lib/IR/Verifier.cpp:601
@@ +600,3 @@
+  // GlobalValue with the same name as the Comdat.
+  switch (C.getSelectionKind()) {
+  case Comdat::Any:
----------------
Rafael Ávila de Espíndola wrote:
> nit:
> 
> C.getSelectionKind() != omdat::Any
> 
Done.

================
Comment at: lib/Linker/LinkModules.cpp:599
@@ +598,3 @@
+      return emitError("Linking COMDATs named '" + ComdatName +
+                       "': COMDAT key involves incomputable alias size.");
+
----------------
Rafael Ávila de Espíndola wrote:
> needs test.
Done.

================
Comment at: test/MC/COFF/comdat.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple i386-pc-win32 < %s | FileCheck %s
+
----------------
Rafael Ávila de Espíndola wrote:
> This should probably be in codegen, no MC.
Done.

================
Comment at: test/MC/ELF/comdat.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple x86_64-pc-linux-gnu < %s | FileCheck %s
+
----------------
Rafael Ávila de Espíndola wrote:
> CodeGen.
Done.

================
Comment at: test/MC/MachO/comdat.ll:1
@@ +1,2 @@
+; RUN: not llc -mtriple x86_64-apple-darwin < %s 2> %t
+; RUN: FileCheck < %t %s
----------------
Rafael Ávila de Espíndola wrote:
> CodeGen
Done.

http://reviews.llvm.org/D4178






More information about the llvm-commits mailing list