[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9c05fc39145: [WebAssembly] Use the new crt1-command.o if 
present. (authored by sunfish).

Changed prior to commit:
  https://reviews.llvm.org/D89274?vs=297698=323164#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,16 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o. This is a temporary transition measure.
+  // Once WASI libc no longer needs to support LLVM versions which lack
+  // support for new-style command, it can make crt1.o the same as
+  // crt1-command.o. And once LLVM no longer needs to support WASI libc
+  // versions before that, it can switch to using crt1-command.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,16 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o. This is a temporary transition measure.
+  // Once WASI libc no longer needs to support LLVM versions which lack
+  // support for new-style command, it can make crt1.o the same as
+  // crt1-command.o. And once LLVM no longer needs to support WASI libc
+  // versions before that, it can switch to using crt1-command.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Sam Clegg via Phabricator via cfe-commits]

sbc100 accepted this revision.
sbc100 added a comment.
This revision is now accepted and ready to land.

Ok so we can see this as an interm thing.  I think I'm OK with that.  Could you 
add a comment about that, or at least say why we want to support both the old 
crt1 and the new crt1-command at the same time (i.e. so that wasi-libc can 
easily suppot both old and new llvm right?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

I don't see a way to do this with weak symbols, and an install script would be 
yet-another moving part that we'd have to make on end-user systems.

How about this: once we reach a point where we don't support the old LLVM 
anymore, libc can make crt1.o be the same as crt1-command.o, and then once 
we're sure we don't support libc old than that, we can switch LLVM back to 
crt1.o :-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

In D89274#211 , @sunfish wrote:

> It's to ensure that older LLVM works with newer WASI libc, and newer clang 
> works with older WASI libc. New-style commands require [lld support]. We can 
> assume that if clang is updated, lld has the requisite support.
>
> That said, I'm open to other ideas here.
>
> [lld support]: 
> https://github.com/llvm/llvm-project/commit/6cd8511e5932e4a53b2bb7780f69489355fc7783

Certainly seems like its would be better if would find a way to keep the crt1.o 
name if possible.

I cant't remember now what the old and new crt1.o looks like... but maybe we 
can find way for new WASI libc to detect if the new linker is being used?  
Perhaps using weak symbol references in crt1.o?

If that isn't possible then perhaps some kind of install script for wasi-libc 
that can install one or the other?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

It's to ensure that older LLVM works with newer WASI libc, and newer clang 
works with older WASI libc. New-style commands require [lld support]. We can 
assume that if clang is updated, lld has the requisite support.

That said, I'm open to other ideas here.

[lld support]: 
https://github.com/llvm/llvm-project/commit/6cd8511e5932e4a53b2bb7780f69489355fc7783


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Why not just use crt1.o in both cases?If you are going to prefer 
crt1-command.o in all cases then a toolchain would have no reason to ever ship 
crt1.o would it?  (since it would always be ignored?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: sbc100.
Herald added subscribers: cfe-commits, ecnelises, jgravelle-google, dschuff.
Herald added a project: clang.
sunfish requested review of this revision.
Herald added a subscriber: aheejin.

If crt1-command.o exists in the sysroot, the libc has new-style command
support, so use it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89274

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,12 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,12 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits