<div dir="ltr">I filed <a href="https://llvm.org/bugs/show_bug.cgi?id=31488">https://llvm.org/bugs/show_bug.cgi?id=31488</a> for this.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 28, 2016 at 9:37 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This affected code generation of jump tables from switches. This was the assembly difference for a switch when going from PIC to static:<div><br></div><div><div>$ cat t.cpp</div><div>void g(int);</div><div>void f(int x) {</div><div>  switch (x) {</div><div>  case 0: g(0); break;</div><div>  case 1: g(1); break;</div><div>  case 2: g(2); break;</div><div>  case 3: g(3); break;</div><div>  case 4: g(4); break;</div><div>  case 5: g(5); break;</div><div>  case 6: g(6); break;</div><div>  case 7: g(7); break;</div><div>  case 8: g(8); break;</div><div>  case 9: g(9); break;</div><div>  }</div><div>  g(10);</div><div>}</div><div><div><br></div><div>$ clang -cc1 t.cpp  -S -o - -munwind-tables -mrelocation-model pic -o pic.s</div><div><br></div><div>$ clang -cc1 t.cpp  -S -o - -munwind-tables -mrelocation-model static -o static.s</div></div><div><br></div><div>$ diff -u pic.s static.s<br></div><div>--- pic.s       2016-12-28 09:32:48.190468800 -0800</div><div>+++ static.s    2016-12-28 09:32:53.476899200 -0800</div><div>@@ -21,11 +21,9 @@</div><div>        movq    %rax, 40(%rsp)</div><div>        movq    %rdx, 32(%rsp)</div><div>        ja      .LBB0_11</div><div>-       leaq    .LJTI0_0(%rip), %rax</div><div>-       movq    40(%rsp), %rcx</div><div>-       movslq  (%rax,%rcx,4), %rdx</div><div>-       addq    %rax, %rdx</div><div>-       jmpq    *%rdx</div><div>+       movq    40(%rsp), %rax</div><div>+       movq    .LJTI0_0(,%rax,8), %rcx</div><div>+       jmpq    *%rcx</div><div> .LBB0_1:</div><div>        xorl    %ecx, %ecx</div><div>        callq   "?g@@YAXH@Z"</div><div>@@ -71,20 +69,21 @@</div><div>        nop</div><div>        addq    $56, %rsp</div><div>        retq</div><div>-       .p2align        2, 0x90</div><div>+       .section        .rdata,"dr"</div><div>+       .p2align        3</div><div> .LJTI0_0:</div><div>-       .long   .LBB0_1-.LJTI0_0</div><div>-       .long   .LBB0_2-.LJTI0_0</div><div>-       .long   .LBB0_3-.LJTI0_0</div><div>-       .long   .LBB0_4-.LJTI0_0</div><div>-       .long   .LBB0_5-.LJTI0_0</div><div>-       .long   .LBB0_6-.LJTI0_0</div><div>-       .long   .LBB0_7-.LJTI0_0</div><div>-       .long   .LBB0_8-.LJTI0_0</div><div>-       .long   .LBB0_9-.LJTI0_0</div><div>-       .long   .LBB0_10-.LJTI0_0</div><div>+       .quad   .LBB0_1</div><div>+       .quad   .LBB0_2</div><div>+       .quad   .LBB0_3</div><div>+       .quad   .LBB0_4</div><div>+       .quad   .LBB0_5</div><div>+       .quad   .LBB0_6</div><div>+       .quad   .LBB0_7</div><div>+       .quad   .LBB0_8</div><div>+       .quad   .LBB0_9</div><div>+       .quad   .LBB0_10</div><div>        .seh_handlerdata</div><div>-       .text</div><div>+       .section        .rdata,"dr"</div><div> .Lcfi3:</div><div>        .seh_endproc</div></div><div><br></div><div>I think we actually want the .rdata section, but we probably want to use ".long .LBB0_N@IMGREL" jump table entries.</div><div><br></div><div>This triggers a latent bug in unwind info emission, because .seh_endproc is in the wrong section (.rdata, not .text).</div><div><br></div><div>Let's revert, fix the latent bug, make switch lowering do what we want, and then re-land.</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 28, 2016 at 9:10 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I suspect this broke Win64:<div><a href="http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/1410" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-x64-ninja-win7/build<wbr>s/1410</a><br></div></div><div class="m_-4365090658023417301HOEnZb"><div class="m_-4365090658023417301h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 26, 2016 at 6:20 PM, Saleem Abdulrasool via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: compnerd<br>
Date: Mon Dec 26 20:20:35 2016<br>
New Revision: 290569<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=290569&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=290569&view=rev</a><br>
Log:<br>
Driver: switch Windows to static RelocModel<br>
<br>
Windows uses PE/COFF which is inherently position independent.  The use<br>
of the PIC model is unnecessary.  In fact, we would generate invalid<br>
code using the ELF PIC model when PIC was enabled previously.  Now that<br>
we no longer accept -fPIC and -fpoc, this switches the internal<br>
representation to the static model to permit us to make PIC modules<br>
invalid when targeting Windows.  This should not change the code<br>
generation, only the internal state management.<br>
<br>
Modified:<br>
    cfe/trunk/lib/Driver/MSVCToolC<wbr>hain.cpp<br>
    cfe/trunk/lib/Driver/ToolChain<wbr>s.cpp<br>
    cfe/trunk/lib/Driver/ToolChain<wbr>s.h<br>
    cfe/trunk/test/Driver/pic.c<br>
<br>
Modified: cfe/trunk/lib/Driver/MSVCToolC<wbr>hain.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/MSVCToolChain.cpp?rev=290569&r1=290568&r2=290569&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Driver/MSV<wbr>CToolChain.cpp?rev=290569&r1=2<wbr>90568&r2=290569&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Driver/MSVCToolC<wbr>hain.cpp (original)<br>
+++ cfe/trunk/lib/Driver/MSVCToolC<wbr>hain.cpp Mon Dec 26 20:20:35 2016<br>
@@ -82,18 +82,6 @@ bool MSVCToolChain::IsUnwindTablesD<wbr>efaul<br>
   return getArch() == llvm::Triple::x86_64;<br>
 }<br>
<br>
-bool MSVCToolChain::isPICDefault() const {<br>
-  return getArch() == llvm::Triple::x86_64;<br>
-}<br>
-<br>
-bool MSVCToolChain::isPIEDefault() const {<br>
-  return false;<br>
-}<br>
-<br>
-bool MSVCToolChain::isPICDefaultFor<wbr>ced() const {<br>
-  return getArch() == llvm::Triple::x86_64;<br>
-}<br>
-<br>
 #ifdef USE_WIN32<br>
 static bool readFullStringValue(HKEY hkey, const char *valueName,<br>
                                 std::string &value) {<br>
<br>
Modified: cfe/trunk/lib/Driver/ToolChain<wbr>s.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=290569&r1=290568&r2=290569&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Driver/Too<wbr>lChains.cpp?rev=290569&r1=2905<wbr>68&r2=290569&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Driver/ToolChain<wbr>s.cpp (original)<br>
+++ cfe/trunk/lib/Driver/ToolChain<wbr>s.cpp Mon Dec 26 20:20:35 2016<br>
@@ -2880,22 +2880,13 @@ bool Generic_GCC::IsUnwindTablesDef<wbr>ault(<br>
<br>
 bool Generic_GCC::isPICDefault() const {<br>
   switch (getArch()) {<br>
-  case llvm::Triple::x86_64:<br>
-    return getTriple().isOSWindows();<br>
+  default: return false;<br>
   case llvm::Triple::ppc64:<br>
   case llvm::Triple::ppc64le:<br>
     return !getTriple().isOSBinFormatMach<wbr>O() && !getTriple().isMacOSX();<br>
-  default:<br>
-    return false;<br>
   }<br>
 }<br>
<br>
-bool Generic_GCC::isPIEDefault() const { return false; }<br>
-<br>
-bool Generic_GCC::isPICDefaultForce<wbr>d() const {<br>
-  return getArch() == llvm::Triple::x86_64 && getTriple().isOSWindows();<br>
-}<br>
-<br>
 bool Generic_GCC::IsIntegratedAssem<wbr>blerDefault() const {<br>
   switch (getTriple().getArch()) {<br>
   case llvm::Triple::x86:<br>
<br>
Modified: cfe/trunk/lib/Driver/ToolChain<wbr>s.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=290569&r1=290568&r2=290569&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Driver/Too<wbr>lChains.h?rev=290569&r1=290568<wbr>&r2=290569&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Driver/ToolChain<wbr>s.h (original)<br>
+++ cfe/trunk/lib/Driver/ToolChain<wbr>s.h Mon Dec 26 20:20:35 2016<br>
@@ -232,8 +232,8 @@ public:<br>
<br>
   bool IsUnwindTablesDefault() const override;<br>
   bool isPICDefault() const override;<br>
-  bool isPIEDefault() const override;<br>
-  bool isPICDefaultForced() const override;<br>
+  bool isPIEDefault() const override { return false; }<br>
+  bool isPICDefaultForced() const override { return false; }<br>
   bool IsIntegratedAssemblerDefault() const override;<br>
   llvm::opt::DerivedArgList *<br>
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,<br>
@@ -1136,9 +1136,9 @@ public:<br>
<br>
   bool IsIntegratedAssemblerDefault() const override;<br>
   bool IsUnwindTablesDefault() const override;<br>
-  bool isPICDefault() const override;<br>
-  bool isPIEDefault() const override;<br>
-  bool isPICDefaultForced() const override;<br>
+  bool isPICDefault() const override { return false; }<br>
+  bool isPIEDefault() const override { return false; }<br>
+  bool isPICDefaultForced() const override { return false; }<br>
<br>
   void<br>
   AddClangSystemIncludeArgs(con<wbr>st llvm::opt::ArgList &DriverArgs,<br>
<br>
Modified: cfe/trunk/test/Driver/pic.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pic.c?rev=290569&r1=290568&r2=290569&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Driver/pi<wbr>c.c?rev=290569&r1=290568&r2=29<wbr>0569&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Driver/pic.c (original)<br>
+++ cfe/trunk/test/Driver/pic.c Mon Dec 26 20:20:35 2016<br>
@@ -255,9 +255,4 @@<br>
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1<br>
 // RUN: %clang -c %s -target arm64-linux-android -### 2>&1 \<br>
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1<br>
-//<br>
-// On Windows-X64 PIC is enabled by default<br>
-// RUN: %clang -c %s -target x86_64-pc-windows-msvc18.0.0 -### 2>&1 \<br>
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2<br>
-// RUN: %clang -c %s -target x86_64-pc-windows-gnu -### 2>&1 \<br>
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2<br>
+<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>