[PATCH] D80591: Patch up issues with GN builds (pthread / libz)

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/utils/gn/build/libs/pthread/BUILD.gn:5
   visibility = [ ":pthread" ]
   libs = [ "pthread" ]
 }
----------------
We shouldn't need both `-pthread` and `-lpthread`. Can you remove this one?


================
Comment at: llvm/utils/gn/build/libs/pthread/BUILD.gn:15
   # On Android, bionic has built-in support for pthreads.
   if (llvm_enable_threads && current_os != "win" && current_os != "android") {
     public_configs = [ ":pthread_config" ]
----------------
The ` && current_os != "android"` part should now be unnecessary with the change above assuming that `-pthread` does nothing in the Android driver.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:91
       } else {
-        command = "$ld {{ldflags}} -o $outfile {{libs}} -Wl,--start-group {{inputs}} -Wl,--end-group"
+        command = "$ld {{ldflags}} -o $outfile -Wl,--start-group {{inputs}} {{libs}} -Wl,--end-group"
       }
----------------
Nit: you could keep `{{libs}}` and `{{inputs}}` in the same order as on Mac.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80591/new/

https://reviews.llvm.org/D80591