[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