<div dir="ltr">Ping?</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Mar 7, 2014 at 5:39 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Bigcheese, shankarke, kledzik,<br>
<br>
COMDAT_SELECT_LARGEST is a COMDAT type that make linker to choose the largest<br>
definition from among all of the definition of a symbol. If the size is the<br>
same, the choice is arbitrary.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D3011" target="_blank">http://llvm-reviews.chandlerc.com/D3011</a><br>
<br>
Files:<br>
include/lld/Core/DefinedAtom.h<br>
lib/Core/SymbolTable.cpp<br>
lib/ReaderWriter/PECOFF/ReaderCOFF.cpp<br>
lib/ReaderWriter/YAML/ReaderWriterYAML.cpp<br>
test/pecoff/Inputs/merge-largest1.obj.yaml<br>
test/pecoff/Inputs/merge-largest2.obj.yaml<br>
test/pecoff/merge-largest.test<br>
<br>
Index: include/lld/Core/DefinedAtom.h<br>
===================================================================<br>
--- include/lld/Core/DefinedAtom.h<br>
+++ include/lld/Core/DefinedAtom.h<br>
@@ -99,6 +99,8 @@<br>
mergeAsWeakAndAddressUsed, // Is C++ definition inline definition whose<br>
// address was taken.<br>
mergeSameNameAndSize, // Another atom with different size is error<br>
+ mergeLargest, // Choose the largest one from atoms with the same<br>
+ // name<br>
mergeByContent, // Merge with other constants with same content.<br>
};<br>
<br>
Index: lib/Core/SymbolTable.cpp<br>
===================================================================<br>
--- lib/Core/SymbolTable.cpp<br>
+++ lib/Core/SymbolTable.cpp<br>
@@ -102,13 +102,14 @@<br>
MCR_Error<br>
};<br>
<br>
-static MergeResolution mergeCases[][5] = {<br>
- // no tentative weak weakAddress sameNameAndSize<br>
- {MCR_Error, MCR_First, MCR_First, MCR_First, MCR_SameSize}, // no<br>
- {MCR_Second, MCR_Largest, MCR_Second, MCR_Second, MCR_SameSize}, // tentative<br>
- {MCR_Second, MCR_First, MCR_First, MCR_Second, MCR_SameSize}, // weak<br>
- {MCR_Second, MCR_First, MCR_First, MCR_First, MCR_SameSize}, // weakAddress<br>
- {MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize}, // sameSize<br>
+static MergeResolution mergeCases[][6] = {<br>
+ // no tentative weak weakAddress sameNameAndSize largest<br>
+ {MCR_Error, MCR_First, MCR_First, MCR_First, MCR_SameSize, MCR_Largest}, // no<br>
+ {MCR_Second, MCR_Largest, MCR_Second, MCR_Second, MCR_SameSize, MCR_Largest}, // tentative<br>
+ {MCR_Second, MCR_First, MCR_First, MCR_Second, MCR_SameSize, MCR_Largest}, // weak<br>
+ {MCR_Second, MCR_First, MCR_First, MCR_First, MCR_SameSize, MCR_Largest}, // weakAddress<br>
+ {MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize}, // sameSize<br>
+ {MCR_Largest, MCR_Largest, MCR_Largest, MCR_Largest, MCR_SameSize, MCR_Largest}, // largest<br>
};<br>
<br>
static MergeResolution mergeSelect(DefinedAtom::Merge first,<br>
@@ -148,19 +149,22 @@<br>
case MCR_Second:<br>
useNew = true;<br>
break;<br>
- case MCR_Largest:<br>
- useNew = true;<br>
+ case MCR_Largest: {<br>
+ uint64_t existingSize = ((DefinedAtom*)existing)->size();<br>
+ uint64_t newSize = ((DefinedAtom*)&newAtom)->size();<br>
+ useNew = (newSize >= existingSize);<br>
break;<br>
+ }<br>
case MCR_SameSize: {<br>
- uint64_t sa = ((DefinedAtom*)existing)->size();<br>
- uint64_t sb = ((DefinedAtom*)&newAtom)->size();<br>
- if (sa == sb) {<br>
+ uint64_t existingSize = ((DefinedAtom*)existing)->size();<br>
+ uint64_t newSize = ((DefinedAtom*)&newAtom)->size();<br>
+ if (existingSize == newSize) {<br>
useNew = true;<br>
break;<br>
}<br>
llvm::errs() << "Size mismatch: "<br>
- << existing->name() << " (" << sa << ") "<br>
- << newAtom.name() << " (" << sb << ")\n";<br>
+ << existing->name() << " (" << existingSize << ") "<br>
+ << newAtom.name() << " (" << newSize << ")\n";<br>
// fallthrough<br>
}<br>
case MCR_Error:<br>
Index: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp<br>
===================================================================<br>
--- lib/ReaderWriter/PECOFF/ReaderCOFF.cpp<br>
+++ lib/ReaderWriter/PECOFF/ReaderCOFF.cpp<br>
@@ -253,8 +253,9 @@<br>
return DefinedAtom::mergeByContent;<br>
case llvm::COFF::IMAGE_COMDAT_SELECT_SAME_SIZE:<br>
return DefinedAtom::mergeSameNameAndSize;<br>
- case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:<br>
case llvm::COFF::IMAGE_COMDAT_SELECT_LARGEST:<br>
+ return DefinedAtom::mergeLargest;<br>
+ case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:<br>
case llvm::COFF::IMAGE_COMDAT_SELECT_NEWEST:<br>
// FIXME: These attributes has more complicated semantics than the regular<br>
// weak symbol. These are mapped to mergeAsWeakAndAddressUsed for now<br>
Index: lib/ReaderWriter/YAML/ReaderWriterYAML.cpp<br>
===================================================================<br>
--- lib/ReaderWriter/YAML/ReaderWriterYAML.cpp<br>
+++ lib/ReaderWriter/YAML/ReaderWriterYAML.cpp<br>
@@ -340,6 +340,7 @@<br>
io.enumCase(value, "by-content", lld::DefinedAtom::mergeByContent);<br>
io.enumCase(value, "same-name-and-size",<br>
lld::DefinedAtom::mergeSameNameAndSize);<br>
+ io.enumCase(value, "largest", lld::DefinedAtom::mergeLargest);<br>
}<br>
};<br>
<br>
Index: test/pecoff/Inputs/merge-largest1.obj.yaml<br>
===================================================================<br>
--- /dev/null<br>
+++ test/pecoff/Inputs/merge-largest1.obj.yaml<br>
@@ -0,0 +1,25 @@<br>
+---<br>
+header:<br>
+ Machine: IMAGE_FILE_MACHINE_I386<br>
+ Characteristics: [ ]<br>
+sections:<br>
+ - Name: .text<br>
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]<br>
+ Alignment: 16<br>
+ SectionData: 00112233<br>
+symbols:<br>
+ - Name: .text<br>
+ Value: 0<br>
+ SectionNumber: 1<br>
+ SimpleType: IMAGE_SYM_TYPE_NULL<br>
+ ComplexType: IMAGE_SYM_DTYPE_NULL<br>
+ StorageClass: IMAGE_SYM_CLASS_STATIC<br>
+ NumberOfAuxSymbols: 1<br>
+ AuxiliaryData: 0700000000000000C979F796000006000000<br>
+ - Name: "_foo"<br>
+ Value: 0<br>
+ SectionNumber: 1<br>
+ SimpleType: IMAGE_SYM_TYPE_NULL<br>
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION<br>
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL<br>
+...<br>
Index: test/pecoff/Inputs/merge-largest2.obj.yaml<br>
===================================================================<br>
--- /dev/null<br>
+++ test/pecoff/Inputs/merge-largest2.obj.yaml<br>
@@ -0,0 +1,25 @@<br>
+---<br>
+header:<br>
+ Machine: IMAGE_FILE_MACHINE_I386<br>
+ Characteristics: [ ]<br>
+sections:<br>
+ - Name: .text<br>
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]<br>
+ Alignment: 16<br>
+ SectionData: 0011223344556677<br>
+symbols:<br>
+ - Name: .text<br>
+ Value: 0<br>
+ SectionNumber: 1<br>
+ SimpleType: IMAGE_SYM_TYPE_NULL<br>
+ ComplexType: IMAGE_SYM_DTYPE_NULL<br>
+ StorageClass: IMAGE_SYM_CLASS_STATIC<br>
+ NumberOfAuxSymbols: 1<br>
+ AuxiliaryData: 0700000000000000C979F796000006000000<br>
+ - Name: "_foo"<br>
+ Value: 0<br>
+ SectionNumber: 1<br>
+ SimpleType: IMAGE_SYM_TYPE_NULL<br>
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION<br>
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL<br>
+...<br>
Index: test/pecoff/merge-largest.test<br>
===================================================================<br>
--- /dev/null<br>
+++ test/pecoff/merge-largest.test<br>
@@ -0,0 +1,22 @@<br>
+# RUN: yaml2obj %p/Inputs/merge-largest1.obj.yaml > %t1.obj<br>
+# RUN: yaml2obj %p/Inputs/merge-largest2.obj.yaml > %t2.obj<br>
+#<br>
+# RUN: lld -flavor link /out:%t.exe /subsystem:console /opt:noref /force \<br>
+# RUN: -- %t1.obj %t2.obj > %t.log 2>&1<br>
+# RUN: echo foo >> %t.log<br>
+# RUN: FileCheck -check-prefix=OK %s < %t.log<br>
+#<br>
+# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=READOBJ %s<br>
+<br>
+OK-NOT: duplicate symbol error<br>
+<br>
+READOBJ: Format: COFF-i386<br>
+READOBJ-NEXT: Arch: i386<br>
+READOBJ-NEXT: AddressSize: 32bit<br>
+READOBJ-NEXT: Sections [<br>
+READOBJ-NEXT: Section {<br>
+READOBJ-NEXT: Number: 1<br>
+READOBJ-NEXT: Name: .text (2E 74 65 78 74 00 00 00)<br>
+READOBJ-NEXT: VirtualSize: 0x8<br>
+READOBJ-NEXT: VirtualAddress: 0x1000<br>
+READOBJ-NEXT: RawDataSize: 8<br>
</blockquote></div><br></div>