Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 16 pull requests #58664

Closed
wants to merge 65 commits into from
Closed

Rollup of 16 pull requests #58664

wants to merge 65 commits into from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Feb 23, 2019

Successful merges:

Failed merges:

r? @ghost

matthieu-m and others added 30 commits February 3, 2019 16:58
Specialize Iterator::try_fold and DoubleEndedIterator::try_rfold to
improve code generation in all internal iteration scenarios.

This changes brings the performance of internal iteration with
RangeInclusive on par with the performance of iteration with Range:

 - Single conditional jump in hot loop,
 - Unrolling and vectorization,
 - And even Closed Form substitution.

Unfortunately, it only applies to internal iteration. Despite various
attempts at stream-lining the implementation of next and next_back,
LLVM has stubbornly refused to optimize external iteration
appropriately, leaving me with a choice between:

 - The current implementation, for which Closed Form substitution is
   performed, but which uses 2 conditional jumps in the hot loop when
   optimization fail.
 - An implementation using a "is_done" boolean, which uses 1
   conditional jump in the hot loop when optimization fail, allowing
   unrolling and vectorization, but for which Closed Form substitution
   fails.

In the absence of any conclusive evidence as to which usecase matters
most, and with no assurance that the lack of Closed Form substitution
is not indicative of other optimizations being foiled, there is no way
to pick one implementation over the other, and thus I defer to the
statu quo as far as next and next_back are concerned.
if security_qos_flags(SECURITY_ANONYMOUS) is set
It is currently a method of `Token`, but it only is valid to call if
`self` is a `Token::Interpolated`. This commit eliminates the
possibility of misuse by changing it to an associated function that
takes a `Nonterminal`, which also simplifies the call sites.

This requires splitting out a new function, `nonterminal_to_string`.
It's present within `Token::Interpolated` as an optimization, so that if
a nonterminal is converted to a `TokenStream` multiple times, the
first-computed value is saved and reused.

But in practice it's not needed. `interpolated_to_tokenstream()` is a
cold function: it's only called a few dozen times while compiling rustc
itself, and a few hundred times across the entire `rustc-perf` suite.
Furthermore, when it is called, it is almost always the first
conversion, so no benefit is gained from it.

So this commit removes `LazyTokenStream`, along with the now-unnecessary
`Token::interpolated()`.

As well as a significant simplification, the removal speeds things up
slightly, mostly due to not having to `drop` the `LazyTokenStream`
instances.
The current code (expensively) clones the value within an `Rc`. This
commit changes things so that the `Rc` itself is (cheaply) cloned
instead, avoid some allocations.

This requires converting a few `Rc` instances to `Lrc`.
RReverser and others added 22 commits February 21, 2019 23:02
Nowadays booleans have a well-defined representation, so there is no reason not to optimise their allocation.
…simple.rs test.

(and now unignore the test since it shouldn't break tests of
cross-compiles anymore.)
AFAICT, we do not have the same const-eval issues that we used to when
rust-lang#23926 was filed. (Probably because of the switch to
miri for const-evaluation.)
RangeInclusive internal iteration performance improvement.

Specialize `Iterator::try_fold` and `DoubleEndedIterator::try_rfold` to improve code generation in all internal iteration scenarios.

This changes brings the performance of internal iteration with `RangeInclusive` on par with the performance of iteration with `Range`:

 - Single conditional jump in hot loop,
 - Unrolling and vectorization,
 - And even Closed Form substitution.

Unfortunately, it only applies to internal iteration. Despite various attempts at stream-lining the implementation of `next` and `next_back`, LLVM has stubbornly refused to optimize external iteration appropriately, leaving me with a choice between:

 - The current implementation, for which Closed Form substitution is performed, but which uses 2 conditional jumps in the hot loop when optimization fail.
 - An implementation using a `is_done` boolean, which uses 1 conditional jump in the hot loop when optimization fail, allowing unrolling and vectorization, but for which Closed Form substitution fails.

In the absence of any conclusive evidence as to which usecase matters most, and with no assurance that the lack of Closed Form substitution is not indicative of other optimizations being foiled, there is no way
to pick one implementation over the other, and thus I defer to the statu quo as far as `next` and `next_back` are concerned.
…stebank

Add better error message for partial move

closes rust-lang#56657

r? @davidtwco
 Set secure flags when opening a named pipe on Windows

Fixes rust-lang#42036, see also the previous attempt in rust-lang#44556.

Whether this is correct depends on if it is somehow possible to create a symlink to a named pipe, outside the named pipe filesystem (NPFS). But as far as I can tell that should be impossible.

Also fixes that `security_qos_flags(SECURITY_ANONYMOUS)` does not set the `SECURITY_SQOS_PRESENT` flag, and the incorrect documentation about the default value of `security_qos_flags`.
Implement unstable ffi_return_twice attribute

This PR implements [RFC2633](rust-lang/rfcs#2633)

r? @eddyb
…nts, r=arielb1

Check the Self-type of inherent associated constants

r? @arielb1
SGX target: fix panic = abort

What is the difference between `no_mangle` and `rustc_std_internal_symbol`?
…hton

Refactor Windows stdio and remove stdin double buffering

I was looking for something nice and small to work on, tried to tackle a few FIXME's in Windows stdio, and things grew from there.

This part of the standard library contains some tricky code, and has changed over the years to handle more corner cases. It could use some refactoring and extra comments.

Changes/fixes:
- Made `StderrRaw` `pub(crate)`, to remove the `Write` implementations on `sys::Stderr` (used unsynchronised for panic output).
- Remove the unused `Read` implementation on `sys::windows::stdin`
- The `windows::stdio::Output` enum made sense when we cached the handles, but we can use simple functions like `is_console` now that we get the handle on every read/write
- `write` can now calculate the number of written bytes as UTF-8 when we can't write all `u16`s.
- If `write` could only write one half of a surrogate pair, attempt another write for the other because user code can't reslice in any way that would allow us to write it otherwise.
- Removed the double buffering on stdin. Documentation on the unexposed `StdinRaw` says: 'This handle is not synchronized or buffered in any fashion'; which is now true.
- `sys::windows::Stdin` now always only partially fills its buffer, so we can guarantee any arbitrary UTF-16 can be re-encoded without losing any data.
- `sys::windows::STDIN_BUF_SIZE` is slightly larger to compensate. There should be no real change in the number of syscalls the buffered `Stdin` does. This buffer is a little larger, while the extra buffer on Stdin is gone.
- `sys::windows::Stdin` now attempts to handle unpaired surrogates at its buffer boundary.
- `sys::windows::Stdin` no langer allocates for its buffer, but the UTF-16 decoding still does.

### Testing
I did some manual testing of reading and writing to console. The console does support UTF-16 in some sense, but doesn't supporting displaying characters outside the BMP.
- compile stage 1 stdlib with a tiny value for `MAX_BUFFER_SIZE` to make it easier to catch corner cases
- run a simple test program that reads on stdin, and echo's to stdout
- write some lines with plenty of ASCII and emoji in a text editor
- copy and paste in console to stdin
- return with `\r\n\` or CTRL-Z
- copy and paste in text editor
- check it round-trips

-----

Fixes rust-lang#23344. All but one of the suggestions in that issue are now implemented. the missing one is:

> * When reading data, we require the entire set of input to be valid UTF-16. We should instead attempt to read as much of the input as possible as valid UTF-16, only returning an error for the actual invalid elements. For example if we read 10 elements, 5 of which are valid UTF-16, the 6th is bad, and then the remaining are all valid UTF-16, we should probably return the first 5 on a call to `read`, then return an error, then return the remaining on the next call to `read`.

Stdin in Console mode is dealing with text directly input by a user. In my opinion getting an unpaired surrogate is quite unlikely in that case, and a valid reason to error on the entire line of input (which is probably short). Dealing with it is incompatible with an unbuffered stdin, which seems the more interesting guarantee to me.
…etrochenkov

Remove `LazyTokenStream`.

`LazyTokenStream` was added in rust-lang#40939. Perhaps it was an effective optimization then, but no longer. This PR removes it, making the code both simpler and faster.

r? @alexcrichton
…iated, r=oli-obk

Turn duration consts into associated consts

As suggested in rust-lang#57391 (comment), I'm moving `Duration` constants (`SECOND`, `MILLISECOND` and so on; currently behind unstable `duration_constants` feature) into the `impl Duration` block.

cc @frewsxcv @SimonSapin
Allow Self::Module to be mutated.

This only changes `&Self::Module` to `&mut Self::Module` in a couple of places.

`codegen_allocator` and `write_metadata` from `ExtraBackendMethods` mutate the underlying LLVM module. As such, it makes sense for these two functions to receive a mutable reference to the module (as opposed to an immutable one).

I am trying to implement `codegen_allocator` for my backend, and I need to be able to mutate `Self::Module`:
https://github.com/rust-lang/rust/blob/f66e4697ae286985ddefc53c3a047614568458bb/src/librustc_codegen_ssa/traits/backend.rs#L41
Modifying the module in `codegen_allocator`/`write_metadata` is not a problem for the LLVM backend, because [ModuleLlvm](https://github.com/rust-lang/rust/blob/master/src/librustc_codegen_llvm/lib.rs#L357) contains a raw pointer to the underlying LLVM module, so it can easily be mutated through FFI calls.

I am trying to avoid interior mutability and `unsafe` as much as I can. What do you think? Does this change make sense, or is there a reason why this should stay the way it is?
…-obk

Optimise vec![false; N] to zero-alloc

Nowadays booleans have a well-defined representation, so there is no reason not to optimise their allocation.
… r=GuillaumeGomez

rustdoc: support methods on primitives in intra-doc links

Fixes rust-lang#58598.
…anishearth

Don't generate minification variables if minification disabled

If the minification is disabled, there is no sense having those variables.

r? @QuietMisdreavus
…r=nikomatsakis

Update tests to account for cross-platform testing and miri.

Fix rust-lang#23926
Do not underflow after resetting unmatched braces count

Fix rust-lang#58638.

r? @oli-obk
@Centril
Copy link
Contributor Author

Centril commented Feb 23, 2019

@bors r+ p=16

@bors
Copy link
Contributor

bors commented Feb 23, 2019

📌 Commit de703e7 has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 23, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:08cf9e22:start=1550882774020793592,finish=1550882849640715625,duration=75619922033
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---

[00:03:50] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:51] tidy error: duplicate error code: 723
[00:03:51] tidy error: /checkout/src/librustc_mir/diagnostics.rs:2380: E0723: r##"
[00:03:51] tidy error: /checkout/src/librustc_typeck/diagnostics.rs:4741:     E0723, // `#[ffi_returns_twice]` is only allowed in foreign functions
[00:03:52] some tidy checks failed
[00:03:52] 
[00:03:52] 
[00:03:52] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:52] 
[00:03:52] 
[00:03:52] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:52] Build completed unsuccessfully in 0:00:47
[00:03:52] Build completed unsuccessfully in 0:00:47
[00:03:52] make: *** [tidy] Error 1
[00:03:52] Makefile:68: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:00577510
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sat Feb 23 00:51:31 UTC 2019
---
travis_time:end:17fcca80:start=1550883092848912239,finish=1550883092854717407,duration=5805168
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1773bb78
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0afce86a
travis_time:start:0afce86a
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0a73e734
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Centril Centril closed this Feb 23, 2019
@Centril Centril deleted the rollup branch February 23, 2019 01:14
@Centril Centril added the rollup A PR which is a rollup label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.