[PATCH] Alternative way of representing comdats

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Thu Jun 26 15:13:02 PDT 2014


Please add 

  $foo = comdat any
  @bar = global i32 42, comdat $foo
  @foo = global i32 42

as an ELF test.

================
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
----------------
The sections or the comdat symbols btw? What does link.exe do?

================
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.
----------------
This is undocumented and it seems unused. How about removing it for now?

================
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.
----------------
The sections or the comdat symbol must be the same size?

================
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:
+
----------------
"selected if the COMDAT's key is the largest."

i.e., we have to drop the "section", no?


================
Comment at: docs/LangRef.rst:785
@@ -719,3 +784,3 @@
 .. _namedmetadatastructure:
 
 Named Metadata
----------------
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


================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:796
@@ +795,3 @@
+  if (!ComdatGV)
+    report_fatal_error("Associative COMDAT symbol '" + ComdatGVName +
+                       "' does not exist.");
----------------
needs test

================
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.");
----------------
needs test.

================
Comment at: lib/IR/Verifier.cpp:601
@@ +600,3 @@
+  // GlobalValue with the same name as the Comdat.
+  switch (C.getSelectionKind()) {
+  case Comdat::Any:
----------------
nit:

C.getSelectionKind() != omdat::Any


================
Comment at: lib/Linker/LinkModules.cpp:599
@@ +598,3 @@
+      return emitError("Linking COMDATs named '" + ComdatName +
+                       "': COMDAT key involves incomputable alias size.");
+
----------------
needs test.

================
Comment at: test/MC/COFF/comdat.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple i386-pc-win32 < %s | FileCheck %s
+
----------------
This should probably be in codegen, no MC.

================
Comment at: test/MC/ELF/comdat.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple x86_64-pc-linux-gnu < %s | FileCheck %s
+
----------------
CodeGen.

================
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
----------------
CodeGen

http://reviews.llvm.org/D4178






More information about the llvm-commits mailing list