Clang toolchain driver for PNaCl

Clang toolchain driver for PNaCl

This is a toolchain driver for PNaCl which aims to replace the
pnacl-clang and pnacl-clang++ Python scripts with a native
version integrated directly into Clang.

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4347
R=dschuff@chromium.org

Committed: ec03bad7cc8a439c8bdfbfc1791139e9f0562b46

[Petr Hosek, 2015-12-22 20:11:01]

Description was changed from

==========
Clang toolchain driver for PNaCl

This is a toolchain driver for PNaCl which aims to replace the
pnacl-clang and pnacl-clang++ Python scripts with a native
version integrated directly into Clang.

BUG=
==========

to

==========
Clang toolchain driver for PNaCl

This is a toolchain driver for PNaCl which aims to replace the
pnacl-clang and pnacl-clang++ Python scripts with a native
version integrated directly into Clang.

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4347
R=dschuff@chromium.org
==========

[Petr Hosek, 2015-12-22 20:11:02]

phosek@chromium.org changed reviewers:
+ dschuff@chromium.org

[Derek Schuff, 2016-02-02 23:19:12]

https://codereview.chromium.org/1547623002/diff/1/include/clang/Driver/ToolCh...
File include/clang/Driver/ToolChain.h (right):

https://codereview.chromium.org/1547623002/diff/1/include/clang/Driver/ToolCh...
include/clang/Driver/ToolChain.h:350: virtual bool isVirtualTarget() const {
return false; }
This should probably have a different name because virtual target would imply
something like NVPTX or wasm; i.e. targets with backends but not physical
hardware. Maybe isBitcodeOnlyTarget?

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.cpp
File lib/Driver/ToolChains.cpp (right):

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.cpp#n...
lib/Driver/ToolChains.cpp:3746: llvm::errs() << "Cannot build assembler for
PNaCl\n";
If this is expected to never happen it should probably be an assert, and if it
could happen based on user input, it should probably be a Diag (at least if it
wouldn't require defining new diag types)

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.h
File lib/Driver/ToolChains.h (right):

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.h#new...
lib/Driver/ToolChains.h:810: llvm::opt::ArgStringList &CC1Args) const override;
length overflow (maybe just clang-format the diff?)

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/Tools.cpp
File lib/Driver/Tools.cpp (right):

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/Tools.cpp#newcod...
lib/Driver/Tools.cpp:4545: getToolChain().getArch() != llvm::Triple::le32))
is this extra condition subsumed by EnableVec?

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/Tools.cpp#newcod...
lib/Driver/Tools.cpp:8239: bool EH = exceptionSettings(Args,
ToolChain.getTriple());
do we have code somewhere that makes -fno-exceptions the default?

[Petr Hosek, 2016-02-03 22:49:27]

https://codereview.chromium.org/1547623002/diff/1/include/clang/Driver/ToolCh...
File include/clang/Driver/ToolChain.h (right):

https://codereview.chromium.org/1547623002/diff/1/include/clang/Driver/ToolCh...
include/clang/Driver/ToolChain.h:350: virtual bool isVirtualTarget() const {
return false; }
On 2016/02/02 23:19:11, Derek Schuff wrote:
> This should probably have a different name because virtual target would imply
> something like NVPTX or wasm; i.e. targets with backends but not physical
> hardware. Maybe isBitcodeOnlyTarget?

Done.

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.cpp
File lib/Driver/ToolChains.cpp (right):

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.cpp#n...
lib/Driver/ToolChains.cpp:3746: llvm::errs() << "Cannot build assembler for
PNaCl\n";
On 2016/02/02 23:19:11, Derek Schuff wrote:
> If this is expected to never happen it should probably be an assert, and if it
> could happen based on user input, it should probably be a Diag (at least if it
> wouldn't require defining new diag types)

Done.

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.h
File lib/Driver/ToolChains.h (right):

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/ToolChains.h#new...
lib/Driver/ToolChains.h:810: llvm::opt::ArgStringList &CC1Args) const override;
On 2016/02/02 23:19:11, Derek Schuff wrote:
> length overflow (maybe just clang-format the diff?)

Done.

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/Tools.cpp
File lib/Driver/Tools.cpp (right):

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/Tools.cpp#newcod...
lib/Driver/Tools.cpp:4545: getToolChain().getArch() != llvm::Triple::le32))
On 2016/02/02 23:19:11, Derek Schuff wrote:
> is this extra condition subsumed by EnableVec?

This was a leftover, removed.

https://codereview.chromium.org/1547623002/diff/1/lib/Driver/Tools.cpp#newcod...
lib/Driver/Tools.cpp:8239: bool EH = exceptionSettings(Args,
ToolChain.getTriple());
On 2016/02/02 23:19:11, Derek Schuff wrote:
> do we have code somewhere that makes -fno-exceptions the default?

I changed the code that enables exception in addExceptionArgs so that exceptions
aren't enabled unless explicitly requested (which should be the same as passing
in "-fno-exceptions"). I also had to add the "-fexceptions" flag to libcxx
build.

[Derek Schuff, 2016-02-24 18:22:30]

looks mostly good

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.cpp
File lib/Driver/ToolChains.cpp (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.c...
lib/Driver/ToolChains.cpp:3746: assert(false && "cannot build assembler");
actually, use llvm_unreachable instead of assert(false)

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.h
File lib/Driver/ToolChains.h (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.h...
lib/Driver/ToolChains.h:778: virtual void anchor();
I don't think we need the anchor since we have an out-of-line constructor.

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp
File lib/Driver/Tools.cpp (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp#ne...
lib/Driver/Tools.cpp:8327: std::string Plugin = ToolChain.getDriver().Dir +
"/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold.so";
Is the file name different on Windows?

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c
File test/CodeGen/pr18235.c (left):

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c#...
test/CodeGen/pr18235.c:1: // RUN: not %clang_cc1 -triple le32-unknown-nacl %s -S
-o - 2>&1 | FileCheck %s
Is there another triple with no target registered that we could change this to?
That would probably make things easier if/when we want to upstream this.

[Petr Hosek, 2016-02-26 00:43:39]

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.cpp
File lib/Driver/ToolChains.cpp (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.c...
lib/Driver/ToolChains.cpp:3746: assert(false && "cannot build assembler");
On 2016/02/24 18:22:29, Derek Schuff wrote:
> actually, use llvm_unreachable instead of assert(false)

Done.

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.h
File lib/Driver/ToolChains.h (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/ToolChains.h...
lib/Driver/ToolChains.h:778: virtual void anchor();
On 2016/02/24 18:22:29, Derek Schuff wrote:
> I don't think we need the anchor since we have an out-of-line constructor.

Done.

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp
File lib/Driver/Tools.cpp (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp#ne...
lib/Driver/Tools.cpp:8327: std::string Plugin = ToolChain.getDriver().Dir +
"/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold.so";
On 2016/02/24 18:22:29, Derek Schuff wrote:
> Is the file name different on Windows?

Done, although I'm not sure if this is the most Clang-ish way to handle this.

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c
File test/CodeGen/pr18235.c (left):

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c#...
test/CodeGen/pr18235.c:1: // RUN: not %clang_cc1 -triple le32-unknown-nacl %s -S
-o - 2>&1 | FileCheck %s
On 2016/02/24 18:22:29, Derek Schuff wrote:
> Is there another triple with no target registered that we could change this
to?
> That would probably make things easier if/when we want to upstream this.

spir?

[Derek Schuff, 2016-02-26 22:36:56]

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp
File lib/Driver/Tools.cpp (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp#ne...
lib/Driver/Tools.cpp:8327: std::string Plugin = ToolChain.getDriver().Dir +
"/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold.so";
On 2016/02/26 00:43:39, Petr Hosek wrote:
> On 2016/02/24 18:22:29, Derek Schuff wrote:
> > Is the file name different on Windows?
> 
> Done, although I'm not sure if this is the most Clang-ish way to handle this.

There is a define LTDL_SHLIB_EXT in include/llvm/Config/config.h

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c
File test/CodeGen/pr18235.c (left):

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c#...
test/CodeGen/pr18235.c:1: // RUN: not %clang_cc1 -triple le32-unknown-nacl %s -S
-o - 2>&1 | FileCheck %s
On 2016/02/26 00:43:38, Petr Hosek wrote:
> On 2016/02/24 18:22:29, Derek Schuff wrote:
> > Is there another triple with no target registered that we could change this
> to?
> > That would probably make things easier if/when we want to upstream this.
> 
> spir?

Yeah, if it works that's fine.

[Petr Hosek, 2016-03-01 22:54:56]

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp
File lib/Driver/Tools.cpp (right):

https://codereview.chromium.org/1547623002/diff/20001/lib/Driver/Tools.cpp#ne...
lib/Driver/Tools.cpp:8327: std::string Plugin = ToolChain.getDriver().Dir +
"/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold.so";
On 2016/02/26 22:36:56, Derek Schuff wrote:
> On 2016/02/26 00:43:39, Petr Hosek wrote:
> > On 2016/02/24 18:22:29, Derek Schuff wrote:
> > > Is the file name different on Windows?
> > 
> > Done, although I'm not sure if this is the most Clang-ish way to handle
this.
> 
> There is a define LTDL_SHLIB_EXT in include/llvm/Config/config.h

Done.

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c
File test/CodeGen/pr18235.c (left):

https://codereview.chromium.org/1547623002/diff/20001/test/CodeGen/pr18235.c#...
test/CodeGen/pr18235.c:1: // RUN: not %clang_cc1 -triple le32-unknown-nacl %s -S
-o - 2>&1 | FileCheck %s
On 2016/02/26 22:36:56, Derek Schuff wrote:
> On 2016/02/26 00:43:38, Petr Hosek wrote:
> > On 2016/02/24 18:22:29, Derek Schuff wrote:
> > > Is there another triple with no target registered that we could change
this
> > to?
> > > That would probably make things easier if/when we want to upstream this.
> > 
> > spir?
> 
> Yeah, if it works that's fine.

Done.

[Derek Schuff, 2016-03-01 23:07:53]

lgtm

[Petr Hosek, 2016-03-07 20:39:02]

Description was changed from

==========
Clang toolchain driver for PNaCl

This is a toolchain driver for PNaCl which aims to replace the
pnacl-clang and pnacl-clang++ Python scripts with a native
version integrated directly into Clang.

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4347
R=dschuff@chromium.org
==========

to

==========
Clang toolchain driver for PNaCl

This is a toolchain driver for PNaCl which aims to replace the
pnacl-clang and pnacl-clang++ Python scripts with a native
version integrated directly into Clang.

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4347
R=dschuff@chromium.org

Committed: ec03bad7cc8a439c8bdfbfc1791139e9f0562b46
==========

[Petr Hosek, 2016-03-07 20:39:03]

Committed patchset #5 (id:80001) manually as
ec03bad7cc8a439c8bdfbfc1791139e9f0562b46 (presubmit successful).