Port Rust to PNaCl/JS (rebase #1)

[DiamondLovesYou]

This is a rebase of #26148 for the LLVM 3.7 changes. See #26148 for the previous set of comments. I've created a new PR so the original comments won't disappear.

[rust-highfive]

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

[bors]

☔ The latest upstream changes (presumably #25784) made this pull request unmergeable. Please resolve the merge conflicts.

[geofft]

Possibly a dumb question, but where should I look for the equivalent of kernel / libc headers in PNaCl? I noticed I collided with you on the signal-handling bindings and I was going to make sure they're correct (there were some wrong ones before on other ports), but I also just want to get a sense of what signal-handling even means on PNaCl. In general it'd be useful to have a pointer to the C headers to verify ABI-compatibility.

(Also this PR is pretty exciting, I'm looking forward to being able to use this.)

[brson]

Thanks @DiamondLovesYou. @alexcrichton or I will review again soon.

[alexcrichton]

Both @brson and I very much want Rust to run everywhere, so we want to facilitate ports of the Rust to various platforms as much as possible. After looking this over again, however, this is going to need to be scaled back to be more conservative to land. The changes made here remain quite invasive, especially when it comes to LLVM bindings and the backend for emitting this code, and unfortunately with the current impact it's just too much to keep around for a niche platform. Ultimately, it's unclear to me whether all the extra overhead here is worth gaining support to target PNaCl. Specifically, my concerns remain along the lines of:

• The amount of work needed to instruct LLVM how to emit code is very large. All other platforms have standard "here's what the platform looks like" but then follow the traditional toolchain paths for assembling code.
• The compiler changes of the form if pnacl { ... } are also very large and aren't well isolated from the rest of the compiler.

I'm very worried about investing so much effort in adding support for an unsupported upstream platform in LLVM. We won't have any automated testing for any of this support, and it seems like it's highly susceptible to going out of date quickly.

I would still love to support PNaCl, however, so I'm curious if any of the following strategies would suffice:

• How far back can these changes be scaled? Is this the absolute minimum needed to compile?
• Can we hold off until the WebAssembly backend is upstreamed in LLVM? From what I know that's basically targeting the same use case as PNaCl, and having an upstream LLVM implementation will also likely mean that it'll be less invasive to add support for.

[DiamondLovesYou]

@geofft From the PNaCl toolchain root (ie pepper_canary/toolchain/linux_pnacl), the signals header is le32-nacl/include/sys/signal.h.

[DiamondLovesYou]

Update: Once https://codereview.chromium.org/1207543003 lands, the changes in librustc_trans/trans will be reverted.

[tomaka]

PNaCl has a lot of similarities with emscripten, so this PR could also open the door to adding support for emscripten!

[DiamondLovesYou]

@tomaka The plan is to use PNaCl as an intermediary, with another tool (possibly in the NaCl SDK) to run the Emscripten passes (ie the passes not run for PNaCl bitcodes) and finally codegen for JS.

[AerialX]

@DiamondLovesYou I'm pretty interested in playing around with this. Could you advise how to compile it? Is a particular llvm + patches required? I've attempted to use pepper_canary and pnacl-llvm/master but rustc just fails to build itself with a FPE.

[DiamondLovesYou]

@AerialX You can't currently. 😦 pnacl-llvm is behind llvm-mirror/llvm@47dfcb7, which is the cause of the FPE you experenced. I've let the NaCl team know (well, JF knows at least), but I don't have write access to their fork, so we have to wait for them to get it merged.

Once that's done however, it might still not work. Because of the aforementioned bug, I haven't been able to test anything PNaCl related.

If you'd still like to target PNaCl, the master branch on my fork might still work with one of the previous pepper versions (ie not canary). Follow the instructions in the readme if you'd like to try.

[DiamondLovesYou]

@alexcrichton @brson Could I get a re-review? There are still a few changes left in pnacl-llvm, but the Rust side changes are for the most part complete. There a few changes I'd like to make in compiletest still, but those are minor.

[brson]

@alexcrichton Re webasm, it does appear that webasm is the inevitable endgame for both emscripten and pnacl, and seems very likely that any support for the latter will be obsoleted by the former within a few years.

@DiamondLovesYou Yes, one of us will take another pass soon.

[DiamondLovesYou]

@brson Alright, thanks!

[alexcrichton]

@DiamondLovesYou This is looking a lot better, but I've still got a number of concerns:

• There's a huge #[cfg] block in the driver. (with no comments)
• New attributes are added to the stable set of Rust.
• New codegen options are added to the compiler.
• A target_libc cfg option has been added.
• A fair bit of C++ code is still being added.

Also, what's the upstream status of this? Is this in upstream LLVM or is a fork still required? I would also be pretty concerned if a fork were still required

[DiamondLovesYou]

@alexcrichton

@DiamondLovesYou This is looking a lot better, but I've still got a number of concerns:

There's a huge #[cfg] block in the driver. (with no comments)

I've added comments. Is that satisfactory?

New attributes are added to the stable set of Rust.

I'm sorry, but the one attribute is necessary so traditional CLI applications can be made to work as a PPAPI plugin and in a way that's compatible with nacl-spawn.

New codegen options are added to the compiler.

Fine; removed.

A target_libc cfg option has been added.

Not really, it's specified as a build flag for le32-unknown-nacl only. As in, crates built with Cargo can't use it.

A fair bit of C++ code is still being added.

Ah, those were leftovers I forgot to remove. Fixed!

Also, what's the upstream status of this? Is this in upstream LLVM or is a fork still required? I would also be pretty concerned if a fork were still required

Strictly speaking, currently, no, a fork is not required. I've made all the changes necessary to use PNaCl's LLVM though, so one can still use it to say reduce the build time. I currently use the prebuilt LLVM from the SDK in my builds because I'm trying to get the Rust compiler into the NaCl SDK.

That said, in the past its been a different story. The difference in LLVM's version between Rust's LLVM && PNaCl's LLVM used to make it quite difficult to use the toolchain with Rust: while LLVM's bitcode format is supposedly stable, that doesn't say anything about the meaning of the bitcode. For example, debugging info metadata.

Anyway, Rust's LLVM is itself a fork, so what's the big fuss?

[AerialX]

Is this usable now? I tried it with pepper_canary's llvm and a simple hello world didn't work out so well.

[DiamondLovesYou]

@AerialX Literally not 15mins ago, I managed to kill the last bug in pnacl-llvm in SimplifyStructRegSignatures.cpp that Rust exposed. I haven't even finished updating the tests, nor sent a patch to the NaCl guys yet. I'm still waiting on a local build of Rust to complete for more comprehensive testing.

It'll be awhile before the forthcoming patch lands in pepper_canary or before Rust will be usable with pnacl-llvm: I'm still waiting on upstream LLVM for this patch to land so I can then cherry that into pnacl-llvm and then so I can fixup this with the refactored function from the aformented upstream patch and finally get a LGTM from JF. And then there's the bug fixes I just completed. After all that, it'll be a couple of days before pepper_canary is updated in the SDK. (You can always build from source though; just follow this guide to checkout out the sources (get llvm with toolchain_build/toolchain_build_pnacl.py --sync-only) then use git cl patch from depot_tools in the LLVM source dir to get my patches, then run toolchain_build/toolchain_build_pnacl.py --install $NACL_SDK_ROOT/toolchain/$($NACL_SDK_ROOT/tools/getos.py)_pnacl to build and install into the SDK install dir).

I appreciate you enthusiasm though! 😃

[AerialX]

Haha, sorry, the way you commented made it sound like it was working with upstream pnacl-llvm :)

I'll see about building my own from source if I get some free time!

[alexcrichton]

I'm sorry, but the one attribute is necessary so traditional CLI applications can be made to work as a PPAPI plugin and in a way that's compatible with nacl-spawn.

This is an attribute in the stable set of rust which will need to be supported until the end of time, these sorts of additions are not taken lightly and we have a process for including new features into the stable set of Rust. Features (like attributes) start behind a feature gate and then later get un-feature-gated.

Not really, it's specified as a build flag for le32-unknown-nacl only. As in, crates built with Cargo can't use it.

Why not just use the normal target_os cfg?

[alexcrichton]

Guh habit of hitting "comment" too soon...

Anyway, Rust's LLVM is itself a fork, so what's the big fuss?

Rust does not require a fork, we have a few small performance-related patches we haven't bothered sending upstream yet, but it's quite important to us that we work with upstream LLVM right now. You can build Rust against the system LLVM for example, and the full test suite should pass (assuming a relatively recent system LLVM).

I'm very wary of adding so much code needed to support a new platform that's (a) on the way out to being deprecated and (b) doesn't work unless you recompile all of Rust against a custom LLVM toolchain.

[bors]

☔ The latest upstream changes (presumably #27210) made this pull request unmergeable. Please resolve the merge conflicts.

[DiamondLovesYou]

@alexcrichton Sorry for the delay.

This is an attribute in the stable set of rust which will need to be supported until the end of time, these sorts of additions are not taken lightly and we have a process for including new features into the stable set of Rust. Features (like attributes) start behind a feature gate and then later get un-feature-gated.

After realizing I am an idiot (yet again!), I just changed it to just use link_name. Problem solved?

Why not just use the normal target_os cfg?

Because Newlib isn't unique to NaCl (even though NaCl's Newlib isn't completely vanilla). In the future, if another port requires use of Newlib, some of the work will already be done. At least that is my intention.

Rust does not require a fork, we have a few small performance-related patches we haven't bothered sending upstream yet, but it's quite important to us that we work with upstream LLVM right now. You can build Rust against the system LLVM for example, and the full test suite should pass (assuming a relatively recent system LLVM).

[...] and (b) doesn't work unless you recompile all of Rust against a custom LLVM toolchain.

Well, by that measure, neither does this PR. You can technically still use rust-llvm with this port (though YMWV by lot). But unfortunately changes in bitcode between different versions of LLVM (despite, as I've heard, their best efforts to keep the BC stable) means that once you try to send bitcode to the toolchain for linking/translating (ie when you try to produce a pexe/bin) you'll very often trip various annoying and inexplicable assertions (note that PNaCl 'object' files are literally just LLVM bitcode). For example, despite the fact both of the LLVMs at issue here are 3.7 (not considering the fact that 3.7 isn't even released yet ಠ_ಠ ), when I rebased this PR the last time, I found pnacl-llvm couldn't read rust-llvm generated BC anymore. One will just get a fatal Invalid value for type error. This is why I say one must use pnacl-llvm, so there is the best possible compatibility between Rust and the PNaCl toolchain irrespective of rust-llvms version. And I believe rust-llvm was even updated again just yesterday.

I suppose I should say 'should' instead of 'must'.

Anyway, this PR still works as expected with upstream LLVM if one doesn't care about targeting PNaCl/JS, the current Travis CI failure notwithstanding.

I'm very wary of adding so much code needed to support a new platform that's (a) on the way out to being deprecated [...]

When the time comes, and wasm descends from the Heavens to become, as foretold in the Great Prophecy, the Lord and Savior of all web developers who'd rather not use JS, I will likely be the one to remove/deprecate the old NaCl code and replace it with wasm code. However, Wasm is at least a year and a half out (even then it'll likely be polyfill only). And JS won't be deprecated until way after that.

[DiamondLovesYou]

Would it help if I split this patch up into less scary PRs that more amenable to code reviewing? It's been mentioned several times that this changeset is huge (even though the net diff now is much less than the total lines +/- reported), so I think if I submitted my work incrementally (instead of submitting it all at once) it would be easier for all of us to agree and would give me more helpful suggestions for what is wanted before code is accepted.

[alexcrichton]

Sorry for being a little slow to respond!

When @brson and I mention that this is a huge PR, it's not so much the size so much as the scope of the changes made here. We have 0 automation for any of the changes beyond that they parse, so there's an very high chance of bitrot for all of this coming in. Most "port to a new platform" is just some definitions in liblibc, tweaking a custom target spec, and possibly adding a few #[cfg] in the standard library. This port, however, involves a number of changes which are more invasive:

• The driver script has picked up a large number of mysterious #[link] annotations which are only used for an untested platform. (is the goal here to run rustc on pnacl?)
• The standard library now has a large split in intrinsics and simd arithmetic for an untested platform.
• Adding a new -C cross-path option
• Lots of is_like_pnacl checks in rustc_trans for linking and code generation
• Changes to alignment, only on pncal

It would be best if this continued the trend of being as minimal as possible when porting to a new platform. For example not making the changes necessary to run the compiler itself on pncal would be a good start (this isn't necessary, right?). Additionally, scaling back as much as possible in the compiler itself to ideally require 0 changes would be best. For example the "if nacl" logic for intrinsics and SIMD operations seems particularly hairy and would be good to simplify.

The size of this PR is fine to me, but breaking up into separate commits is generally quite useful. For example there's a few separable parts you may want to extract:

• Adding support for using a system LLVM which doesn't have all the components we require. This is the makefile logic for scraping components from llvm-config and then only activating what's available in the compiler.
• Splitting apart the backtrace module (without adding nacl support, that's in a separate commit)

(etc)

Some specific changes I would recommend are:

• Don't worry about making first-class error messages for everything that doesn't work on pncal. For example not modifying LTO code, not adding a "no asm!" pass, etc. These will all generate errors anyway, and it's not super important that we have all the support necessary to first-class catch them (because this code is all untested and likely to bitrot).
• Don't change middle::entry to pick up the name of the main function, the #[link_name] attribute should only be read in trans, and even then only in the function that already reads it.
• In general avoid "if pnacl" branches wherever possible. If the cost of leaving out the branch is an obscure error then that's fine, it's just something to avoid in idiomatic pncal. Also if code can compile and work without the branch it's ok to leave out.
• When activating various targets in LLVM, don't bother with stubs for each function that are a noop, instead just make one function that internally has a #[cfg] of whether or not to return early or run all the LLVM functions.

[taralx]

FYI thanks for doing this -- I'm very much looking forward to having this work.

[brson]

I know these reviews have been brutal, but I've given this another once over and taken some unorganized notes, with commentary. My main interest is understanding the intersection of PNaCl, emscripten, and webasm requirements and making sure we don't do too much that is PNaCl specific.

What the PNaCl patch does:

• better out-of-tree llvm support. seems necessary, despite unfortunate build system churn
• pnacl triple and target defs
• teach rustc about pnacl sdk
  • I didn't catch why this was necessary, but there's precedent with the android sdk. We should think about what the 'sdk abstraction' is here and stop coding knowledge of specific sdks into the compiler if we can.
• teach compiletest about pnacl sdk
  • ignore-pnacl
  • lots of churn here. i'd prefer to at least split this out into another patch to focus on the core porting
• adds a chapter to the book!
  • wrong PR for this
• modifies rustc to run under pnacl
  • we don't need to do this. real use cases are slim
• modifies stdlib intrinsic defs to use libm instead
  • this is also done in trans
  • rust is not the place to be polyfilling LLVM intrinsics. LLVM already has facilities for lowering intrinsics to libm. PNaCl should use them.
• polyfills some simd ops
• libc: define nacl-specific stat structure
  • why is stat different here? newlib?
  • might we instead define a 'newbil' platform variant, ala 'musl'
• modifies rustc to disallow asm! for some targets, adds no_asm to target spec
  • making this a pass is weird - could just be treated as any other case where the user provides bogus asm - assembly fails. iow, this seems like a backend function.
• adds is_like_pnacl to target spec
  • used lots of places
  • decide assembler path, why not use existing no_integrated_as?
  • declare some intrinsics as extern C functions with named addresses?!
    • why this and the redirection in std?
    • intrinsics unnamedaddr - why is this needed?
    • shouldn't exist
  • no vectorize_loop/slp - why can't pnacl vectorize?
  • skip codegen
  • max align 8
    • if pnacl can't handle this alignment it should just fix it itself and not require us to frob our bitcode for it
  • this flag should be broken up into flags with feature-specific, not platform-specific meaning. it should not exist
• adds is_lto_supported to target spec
  • why doesn't pnacl support our lto? what happens if somebody tries to lto?
  • since we don't lto by default this branch only happens if somebody asks for lto
• fixes for referencing gdb_debug_scripts under pnacl
• rustdoc flock implementation
  • why is this definition different from other platforms? newlib?
• std: dynamic lib stubs. seems necessary
• std: platform constants in env. ditto
• std: pnacl-definition of stat in std::os. ditto
• std: don't ignore sigpipe
  • why doesn't nacl just handle this silently?
• std: pnacl-specific private data size in unwinder.
• std: turn unwinding personality into an abort...
• std: link to pnacl-specific libs
• std: pnacl-specific ignore dead code in backtrace...
  • this is harmless but ugly. let's find a different solution
• std: stack limit cfgs
• pnacl-specific linker optimization flags
  • what is this optimizing? we've already optimized. this is bad
• why does modules_config.emit_bc get set to true, but there's an explicit branch in write.rs for writing bitcode?

[brson]

I'm feeling like there's a reasonable path forward here, and want to sit down with @alexcrichton and discuss.

[brson]

I've been looking into the existing emscripten ports, posted a patch to update the build tool. I discovered that the std used for that project is really hacked up.

[DiamondLovesYou]

@brson wow those changes w.r.t w/ overflow are using really old macros I wrote for this port a long time ago, and they're actually no longer needed because the PNaCl IR simplification passes now handle them (Emscripten uses most of the PNaCl IR passes itself).

Anyway, I'm working on my reply to your big comment, I should have that finished shortly-ish.

[alexcrichton]

Oh awesome! That'll be a nice chunk that we don't have to deal with any more.

[DiamondLovesYou]

@alexcrichton Be warned though, I have no idea how up-to-date Emscripten's PNaCl passes are. They obviously don't have the changes I've make to pnacl-llvm to remove llvm.assume, among other things.

Emscripten should really, really be targeted via PNaCl though. This is because even pnacl-llvm master can't completely handle Rust IR yet. My local clone can, but I have to do some clean ups for large integer expansion in function signatures before I can send in a patch, plus https://codereview.chromium.org/927493002 which is waiting on http://reviews.llvm.org/D12338 upstream. Oh yeah and http://reviews.llvm.org/D11835.

By using PNaCl, the toolchain can handle the IR simplification and then because it's output will be just bitcode, one can, after adding the .bc extension, send it to emcc. Of course, the usual Emscripten constraints still have to be met (no threading, etc).

Please don't let my apparent focus on PNaCl make one think I don't care about Emscripten.

[DiamondLovesYou]

God I wish quoting people was easier.

• better out-of-tree llvm support. seems necessary, despite unfortunate build system churn

This part was actually split into it's own PR. I have to say though, being able to use a shared object of LLVM is awesome, for the obvious reason that LLVM can be rebuilt without requiring a rebuild of Rust as well.

• pnacl triple and target defs
• teach rustc about pnacl sdk
  • I didn't catch why this was necessary, but there's precedent with the android sdk. We should think about what the 'sdk abstraction' is here and stop coding knowledge of specific sdks into the compiler if we can.

Used to be for convenience. The command line option was actually left on accident and is completely ignored ( @alexcrichton ) . I just forgot to remove it after I removed all the code/logic that used it. I'll get this fixed. PATH is used instead.

• teach compiletest about pnacl sdk
  • ignore-pnacl
  • lots of churn here. i'd prefer to at least split this out into another patch to focus on the core porting

NaCl is technically distinct from PNaCl though. One can target NaCl directly through PNaCl, with the appropriate flags (not in a way that would be valid PNaCl Pexe bitcode, however).
In the future, targeting NaCl could be allowed, permitting things like asm!, zero-cost EH (though that's another story), auto-vectorization, etc, in this way. Which would be useful for e.g. chrome store apps.

I'd like to keep the tests w/ asm! on ignore-pnacl, at least until I have time to refactor things based on whether the target supports asm!, which is obviously more ideal.

compiletest must know about the SDK for similar reasons as why it must know about the Android NDK. Though I will cede that some code could be shared in that regard (it's on the TODO list). compiletest needs to know how to translate PNaCl bitcode into nexe's + how to run the resulting nexe's in the sel_ldr sandbox (unless you're on Linux and have my pexe-runner installed as a binfmt provider), so it's kinda non-optional.

• modifies rustc to run under pnacl
  • we don't need to do this. real use cases are slim

Fair enough, I'll remove them.

• modifies stdlib intrinsic defs to use libm instead
  • this is also done in trans

This doesn't modify stdlib's intrinsic defs to use libm at all. What stdlib does do is to redirect some of the smaller overloads to their larger brethren or to an identical except for type version that libm actually provides. Ie Std only casts. Trans is the only place that actually does the unsupported intrinsics redirection to libm.

... LLVM already has facilities for lowering intrinsics to libm. ...

Not really. While LLVM does have facilities for lowering such intrinsics, such facilities are only available to target machines (via SelectionDAG). Thus PNaCl can't use them.

Yes I have considered creating a target machine for PNaCl. Possible, but a lot of work.

• rust is not the place to be polyfilling LLVM intrinsics. ... PNaCl should use them.

Rust is not the only compiler out there. I appreciate that you want to keep churn at bay (even if I frankly disagree with Rust's aversion to such, given that this is an unsupported port anyway (and an evolving one at that)). I mean no disrespect (really!): I have no indention of dumping this on the Rust team and then just disappearing; rather, I'd like to be considered an 'owner' of these platforms (to this end, my IRC visitation habits could probably use some work; I always forget about IRC), given how long I've been working on this port.

• polyfills some simd ops

An unfortunate kludge; fixable in LLVM but harder and requires more work to do so.

Also this PR is old (that one SIMD rfc was accepted) and these polyfills conflict with the new APIs.

• libc: define nacl-specific stat structure
  • why is stat different here? newlib?
  • might we instead define a 'newlib' platform variant, ala 'musl'

Yes and no. Yes NaCl uses Newlib, but no that wouldn't completely work because NaCl's Newlib isn't vanilla. That said, I thought I set the target details to use 'newlib' for target_env anyway...

• modifies rustc to disallow asm! for some targets, adds no_asm to target spec
  • making this a pass is weird - could just be treated as any other case where the user provides bogus asm - assembly fails. iow, this seems like a backend function.

Except PNaCl, JS, and WAsm don't use an assembler and wouldn't use LLVM's inline assembler expressions anyway. PNaCl has no use case (outside of direct-to-NaCl mentioned above), JS uses emscripten_asm_const (which would probably implemented by a plugin, to keep libsyntax clean), not via LLVM's thing, and WAsm is too new to know for sure. I'm not against asm!, FWIW.

In PNaCl's case, the only manifestation of this error will be a "linker" error (as in pnacl-clang will error), which would confuse users. Plus by printing an error in Rust, one actually knows where the macro is used, whereas no such info would be printed by pnacl-clang.

• adds is_like_pnacl to target spec
  • used lots of places

So are the other is_like_platforms.

• decide assembler path, why not use existing no_integrated_as?

No assembling is ever done (as far as Rust is concerned at least; pnacl-translate does that, but that's a post build step, independent of Rust) in Rust.

• declare some intrinsics as extern C functions with named addresses?!

Huh? Where?

• why this and the redirection in std?

Trans only redirects LLVM intrinsics to their libm equivalents. Intrinsics that can be emulated with a cast live in std.

• intrinsics unnamedaddr - why is this needed?

Hm. I don't remember. I'm checking locally; assuming a couple of PNaCl's run-tests pass, I'll remove it.

• shouldn't exist
• no vectorize_loop/slp - why can't pnacl vectorize?

Because the vectorizer will produce odd sized vector types, ie not 128bit wide. There is no IR level simplification/canonicalization pass to fix this, and it isn't exactly a trivial pass to write (though if (only) PNaCl had a real target machine, SelectionDAG could help out). In fact, it's impossible to do on non-simplified (in the PNaCl IR sense) IR without adding a new instruction (or accepting a perf loss). I have a pass in the works to solve this issue though.

Once finished it'll also be of great benefit to C/C++ PNaCl.

• skip codegen

Until WAsm gets dynamic linking (which is post-MVP), all three of the virtual ISA targets delay codegen until after linking. Both PNaCl && Emscripten use LLVM bitcode as their object format; and I bet WAsm will too, until dynamic linking is added at least.

• max align 8
  • if pnacl can't handle this alignment it should just fix it itself and not require us to frob our bitcode for it this flag should be broken up into flags with feature-specific, not platform-specific meaning. it should not exist adds is_lto_supported to target spec

This isn't a LLVM/PNaCl issue: PNaCl isn't limited to a max alignment of eight. This is an issue of Rust, which embeds zero sized arrays of a type with a specific alignment into trans-ed LLVM structures (for enums I believe) to force alignment. I was the one who originally added the hack to use vector types to force alignment, but unfortunetly the hack can't work when the DataLayout doesn't specify a vector alignment of 128bits (which most other platforms do, apparently).

I haven't tried this yet, but I'll try overriding PNaCl's default DataLayout with a vector alignment of 128bits (ie force the datalayout to have a vector alignment of 128bits). I'll add a comment to update you when I get around to this (I'm in college currently, but I'll get to it when I can).

• why doesn't pnacl support our lto? what happens if somebody tries to lto?

PNaCl's LTO is based on the same LLVM module linking code as Rust's. PNaCl's is much better though (because all deps are bitcode linked, not just Rust deps) and it make's Rust's LTO redundant.

PNaCl/Emscripten is always LTO (well, obviously not O-ed when -O0 is used).

• since we don't lto by default this branch only happens if somebody asks for lto

Not sure what 'this branch' refers to, but Rust's LTO is redundant, so requesting it should be a no-op. FWIW, I'm open to making it a warning on !is_lto_supported. Unless you'd like me to use is_like_pnacl.

• fixes for referencing gdb_debug_scripts under pnacl

I don't think the change is a terrible idea in general though (I'm honestly not sure what LLVM does for these large volatile loads).

• rustdoc flock implementation

Not needed unless rustdoc is hosted in PNaCl (which I have managed to do!), so I can remove.

• std: don't ignore sigpipe
  • why doesn't nacl just handle this silently?

If one considers returning ESYS as handling it, then NaCl does. Unfortunately in PNaCl's case, Rust checks the return value.

• std: turn unwinding personality into an abort...

See the comment in the unwinding personality function. The function is never called.

• std: link to pnacl-specific libs

On my TODO. nacl_io isn't actually needed (it wraps the Pepper API for many Newlib io functions). c++, which is needed for EH, can be remove once PNaCl's EH code is split from libc++/libc++abi.

• std: pnacl-specific ignore dead code in backtrace...
  • this is harmless but ugly. let's find a different solution

Not really all that ugly, considering that I'd have to plaster #[cfg(not(target_os = "nacl"))] everywhere. Which would be way more ugly than current solution (use ... become dead etc)

Is it really that bad for shared implementation helpers to be unused?

At any rate, it is possible to implement crude, non-zero cost, backtracing by having PNaClSjLjEH (a PNaCl IR simplification pass) insert a pointer to a constant string (PNaCl stable bitcode is symbol-less) containing the function name.

• pnacl-specific linker optimization flags
  • what is this optimizing? we've already optimized. this is bad

It's LTO. Literally. PNaCl uses a bitcode linker (ie ld.gold). So passing -O is beneficial as it gets LLVM run over even libc/libm etc, not just Rust code.

• why does modules_config.emit_bc get set to true, but there's an explicit branch in write.rs for writing bitcode?

Must be a leftover I forgot to remove. I'll get it fixed.

---

FYI, I'm splitting this PR up, so it's easier to review.

Damn that was a novel.

[AerialX]

I discovered that the std used for that project is really hacked up.

Note that this was for an overnight PoC, and a lot of it isn't necessary. Getting std in line isn't much of a problem, and doing it properly isn't too much work. The main things emscripten doesn't support are threads+TLS and backtracing.

Not really. While LLVM does have facilities for lowering such intrinsics, such facilities are only available to target machines (via SelectionDAG). Thus PNaCl can't use them.
Yes I have considered creating a target machine for PNaCl. Possible, but a lot of work

And there's the rub, isn't it? It would just make too much sense for these virtual ISAs to expose themselves as a target machine so that downstream compilers wouldn't need any special knowledge about them. If this is so difficult to do (as evidenced by PNaCl using such a hacked-up LLVM), it seems indicative of a problem with LLVM's architecture. One can only hope that wasm ends up being more sane...

In any case, I do believe that getting this working is the way forward for Emscripten to work for the forseeable future. LLVM is too much of a moving target for it to ever work without rustc explicitly being compiled against the same version. The rest (std changes, codegen+trans changes) isn't strictly necessary, but would be nice.

... and I still haven't managed to get rustc/rustllvm to compile against a PNaCl LLVM, all these C++ API changes and DI* -> MD* are mean :(

[DiamondLovesYou]

On Thu, Sep 3, 2015 at 10:11 AM, Aaron Lindsay notifications@github.com
wrote:

I discovered that the std used for that project is really hacked up.

Note that this was for an overnight PoC, and a lot of it isn't necessary.
Getting std in line isn't much of a problem, and doing it properly isn't
too much work. The main things emscripten doesn't support are threads+TLS
and backtracing.

Not really. While LLVM does have facilities for lowering such intrinsics,
such facilities are only available to target machines (via SelectionDAG).
Thus PNaCl can't use them.
Yes I have considered creating a target machine for PNaCl. Possible, but a
lot of work

And there's the rub, isn't it? It would just make too much sense for these
virtual ISAs to expose themselves as a target machine so that downstream
compilers wouldn't need any special knowledge about them. If this is so
difficult to do (as evidenced by PNaCl using such a hacked-up LLVM), it
seems indicative of a problem with LLVM's architecture. One can only hope
that wasm ends up being more sane...

And now you understand one of the motives for WAsm: PNaCl doesn't have/use
a target machine (minus the fact that everyone seems to ignore PNaCl >.<).
Although emscripten might not be what it is currently if PNaCl did.

In any case, I do believe that getting this working is the way forward for
Emscripten to work for the forseeable future. LLVM is too much of a moving
target for it to ever work without rustc explicitly being compiled against
the same version. The rest (std changes, codegen+trans changes) isn't
strictly necessary, but would be nice. I still haven't managed to get
rustc/rustllvm to compile against a PNaCl LLVM though, all these C++ API
changes and DI* -> MD* are mean.

Checkout this file ->
https://github.com/DiamondLovesYou/rust/blob/libstd-pnacl-rebase/src/rustllvm/RustWrapper.cpp
about a third of the way down. You'll need to configure Rust with
CXXFLAGS="-DPNACL_LLVM=1 -nostdinc++ -isystem
/opt/nacl_sdk/pepper_canary/toolchain/linux_pnacl/include/c++/v1" for it to
work.

—
Reply to this email directly or view it on GitHub
#26505 (comment).

[alexcrichton]

From your comment on #28355 it sounds like many of the pieces of this have landed and that's the final piece necessary for pncal support, so closing in favor of that. I can certainly reopen of I'm mistaken though!

[DiamondLovesYou]

Fine with me.

On Sun, Sep 13, 2015 at 12:43 PM, Alex Crichton notifications@github.com
wrote:

From your comment on #28355 #28355
it sounds like many of the pieces of this have landed and that's the final
piece necessary for pncal support, so closing in favor of that. I can
certainly reopen of I'm mistaken though!

—
Reply to this email directly or view it on GitHub
#26505 (comment).