<div dir="ltr">I'm sorry I accidentally submitted this change along with some other change. I'll create another patch now to address the review comments.</div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Mar 7, 2018 at 9:18 AM Rui Ueyama via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This revision was not accepted when it landed; it landed in state "Needs Review".<br>
This revision was automatically updated to reflect the committed changes.<br>
Closed by commit rLLD326911: Improve --warn-symbol-ordering. (authored by ruiu, committed by ).<br>
<br>
Changed prior to commit:<br>
  <a href="https://reviews.llvm.org/D44180?vs=137412&id=137413#toc" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44180?vs=137412&id=137413#toc</a><br>
<br>
Repository:<br>
  rLLD LLVM Linker<br>
<br>
<a href="https://reviews.llvm.org/D44180" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44180</a><br>
<br>
Files:<br>
  ELF/Writer.cpp<br>
  test/ELF/symbol-ordering-file-icf.s<br>
  test/ELF/symbol-ordering-file-warnings.s<br>
<br>
<br>
Index: ELF/Writer.cpp<br>
===================================================================<br>
--- ELF/Writer.cpp<br>
+++ ELF/Writer.cpp<br>
@@ -1045,28 +1045,35 @@<br>
       SymbolOrderEntry &Ent = It->second;<br>
       Ent.Present = true;<br>
<br>
-      auto *D = dyn_cast<Defined>(Sym);<br>
-      if (Config->WarnSymbolOrdering) {<br>
-        if (Sym->isUndefined())<br>
-          warn(File->getName() +<br>
-               ": unable to order undefined symbol: " + Sym->getName());<br>
-        else if (Sym->isShared())<br>
-          warn(File->getName() +<br>
-               ": unable to order shared symbol: " + Sym->getName());<br>
-        else if (D && !D->Section)<br>
-          warn(File->getName() +<br>
-               ": unable to order absolute symbol: " + Sym->getName());<br>
-        else if (D && !D->Section->Live)<br>
-          warn(File->getName() +<br>
-               ": unable to order discarded symbol: " + Sym->getName());<br>
+      auto Warning = [&](StringRef Msg) {<br>
+        if (Config->WarnSymbolOrdering)<br>
+          warn(File->getName() + ": " + Msg + ": " + Sym->getName());<br>
+      };<br>
+<br>
+      if (Sym->isUndefined()) {<br>
+        Warning("unable to order undefined symbol");<br>
+        continue;<br>
       }<br>
-      if (!D)<br>
+      if (Sym->isShared()) {<br>
+        Warning("unable to order shared symbol");<br>
         continue;<br>
+      }<br>
<br>
-      if (auto *Sec = dyn_cast_or_null<InputSectionBase>(D->Section)) {<br>
-        int &Priority = SectionOrder[cast<InputSectionBase>(Sec->Repl)];<br>
-        Priority = std::min(Priority, Ent.Priority);<br>
+      auto *Sec = dyn_cast_or_null<InputSectionBase>(cast<Defined>(Sym)->Section);<br>
+      if (!Sec) {<br>
+        Warning("unable to order absolute symbol");<br>
+        continue;<br>
+      }<br>
+      if (!Sec->Live) {<br>
+        if (Sec->Repl == Sec)<br>
+          Warning("unable to order discarded symbol");<br>
+        else<br>
+          Warning("unable to order a symbol merged by ICF");<br>
+        continue;<br>
       }<br>
+<br>
+      int &Priority = SectionOrder[cast<InputSectionBase>(Sec->Repl)];<br>
+      Priority = std::min(Priority, Ent.Priority);<br>
     }<br>
   }<br>
<br>
Index: test/ELF/symbol-ordering-file-warnings.s<br>
===================================================================<br>
--- test/ELF/symbol-ordering-file-warnings.s<br>
+++ test/ELF/symbol-ordering-file-warnings.s<br>
@@ -109,7 +109,7 @@<br>
 # WARN-NOT:    warning:<br>
 # MISSING:     warning: symbol ordering file: no such symbol: missing<br>
 # MISSING2:    warning: symbol ordering file: no such symbol: missing_sym<br>
-# ICF:         warning: {{.*}}1.o: unable to order discarded symbol: icf2<br>
+# ICF:         warning: {{.*}}1.o: unable to order a symbol merged by ICF: icf2<br>
 # COMDAT:      warning: {{.*}}1.o: unable to order discarded symbol: comdat<br>
 # COMDAT-NEXT: warning: {{.*}}2.o: unable to order discarded symbol: comdat<br>
 # MULTI:       warning: {{.*}}2.o: unable to order absolute symbol: multi<br>
Index: test/ELF/symbol-ordering-file-icf.s<br>
===================================================================<br>
--- test/ELF/symbol-ordering-file-icf.s<br>
+++ test/ELF/symbol-ordering-file-icf.s<br>
@@ -6,11 +6,10 @@<br>
 # RUN: ld.lld --icf=all --symbol-ordering-file %t.order -shared %t.o -o %t.so<br>
 # RUN: llvm-nm %t.so | FileCheck %s<br>
<br>
-## Check that after ICF merges 'foo' and 'zed' we still<br>
-## place them before 'bar', in according to ordering file.<br>
-# CHECK-DAG: 0000000000001000 T foo<br>
-# CHECK-DAG: 0000000000001000 T zed<br>
-# CHECK-DAG: 0000000000001004 T bar<br>
+## Check that we do not sort ICF'ed symbols 'foo' and 'zed'.<br>
+# CHECK-DAG: 0000000000001000 T bar<br>
+# CHECK-DAG: 0000000000001004 T foo<br>
+# CHECK-DAG: 0000000000001004 T zed<br>
<br>
 .section .text.foo,"ax",@progbits<br>
 .align 4<br>
<br>
<br>
</blockquote></div>