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

Rebase to the llvm-project monorepo #57675

Merged
merged 8 commits into from
Jan 26, 2019
Merged

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jan 16, 2019

The new git submodule src/llvm-project is a monorepo replacing src/llvm
and src/tools/{clang,lld,lldb}. This also serves as a rebase for these
projects to the new 8.x branch from trunk.

The src/llvm-emscripten fork is unchanged for now.

r? @alexcrichton

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 16, 2019
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 16, 2019

📌 Commit 50e655e5cf9a8f9e610106a7e612379b61217e8a has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2019
@@ -47,6 +47,7 @@ pub mod libcoretest;
fn filter_dirs(path: &Path) -> bool {
let skip = [
"src/llvm",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably can drop this line?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the LLVM-related src/tools/* here too. My thought was to keep ignoring those paths in tidy, in case a developer still has those obsolete submodules in place. (For instance, they may be moving between branches and don't want to remove them.)

@Centril
Copy link
Contributor

Centril commented Jan 17, 2019

@bors p=3

@alexcrichton
Copy link
Member

@bors: r+

I've added two more commits from rust-lang/llvm-project#2

@bors
Copy link
Contributor

bors commented Jan 17, 2019

📌 Commit aa9d564d917ee7efd998531ac75062feb2e3caba has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 18, 2019

⌛ Testing commit aa9d564d917ee7efd998531ac75062feb2e3caba with merge 5862807814fe0437cd5cf017c09e1b8bb66dc657...

@bors
Copy link
Contributor

bors commented Jan 18, 2019

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 18, 2019
@nikic
Copy link
Contributor

nikic commented Jan 18, 2019

Not really clear to me why it fails to compile core from the AppVeyor log, but possibly this is because compiler-rt in https://github.com/rust-lang-nursery/compiler-builtins hasn't been synced with the LLVM version we're updating to here.

@cuviper
Copy link
Member Author

cuviper commented Jan 18, 2019

Hmm, I wonder if we could get compiler-builtins to just use the monorepo's compiler-rt, so they're always in sync. It would mean the src/llvm-project submodule will always be necessary to checkout, whereas it's currently optional if you're using external LLVM, but that's probably fine for most rustc developers.

Anyway, I do have a machine that I can boot into Windows, so I'll try to reproduce this...

@alexcrichton
Copy link
Member

I think it's probably also fine to just add an llvm-project in the compiler-builtins repository, we can deal with publishing far less to crates.io and git deps hopefully won't be too too common.

@cuviper
Copy link
Member Author

cuviper commented Jan 18, 2019

Oh, I forgot that compiler-builtins is now pulled from crates.io -- then yeah, it could control what it publishes from the monorepo.

Anyway, my first build as-is with MSVC cl was fine. I'm trying again with clang-cl as appveyor does.

@cuviper
Copy link
Member Author

cuviper commented Jan 18, 2019

Well, I can't reproduce the issue with clang-cl either. If anyone else has a ready Windows system to try it, I would appreciate some debugging help.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 20, 2019
@retep998
Copy link
Member

Did you remember to --enable-profiler when trying to reproduce?

@cuviper
Copy link
Member Author

cuviper commented Jan 21, 2019

Yes, I just double-checked config.toml to make sure I had --enable-profiler. It doesn't look like the appveyor log reveals the full configure command, but I guess I could try reproducing more from the given configure output.

Another thing I noticed is appveyor's use of Windows Kits\10\include\10.0.14393.0, whereas I have 10.0.16299.0. I don't know if that would make a difference.

@nikic
Copy link
Contributor

nikic commented Jan 21, 2019

@cuviper Another thing to check would be whether you have assertions=true and verify-llvm-ir=true, as this is an alt build. Unfortunately the non-alt build was cancelled shortly before the point where one could see whether it also fails. Though I don't really see how either of those would result in a silent failure. (Then again, I would expect nothing to result in a silent failure, but here we are.)

@cuviper
Copy link
Member Author

cuviper commented Jan 21, 2019

OK, I matched more config settings, and this time got a dialog about a failed assertion:

image

(If there's a way to get windows/appveyor to show that on the console instead, we should...)

That assertion is:

class MDNode : public Metadata {
//...
  const MDOperand &getOperand(unsigned I) const {
    assert(I < NumOperands && "Out of range");
    return op_begin()[I];
  }

I'll try to get that in a debugger...

@glaubitz
Copy link
Contributor

glaubitz commented Jan 22, 2019

I have already tried building Rust against a newer LLVM version but that fails for me due to LLVM commit eaa73537bb48767af73b2aa8b03eef00d1f6f7c8:

cargo:warning=../rustllvm/RustWrapper.cpp: In function ‘LLVMOpaqueMetadata* LLVMRustDIBuilderCreateFunction(LLVMRustDIBuilderRef, LLVMMetadataRef, const char*, const char*, LLVMMetadataRef, unsigned int, LLVMMetadataRef, bool, bool, unsigned int, LLVMRustDIFlags, bool, LLVMValueRef, LLVMMetadataRef, LLVMMetadataRef)’:
cargo:warning=../rustllvm/RustWrapper.cpp:587:38: error: no matching function for call to ‘llvm::DIBuilder::createFunction(llvm::DIScope*, const char*&, const char*&, llvm::DIFile*, unsigned int&, llvm::DISubroutineType*, bool&, bool&, unsigned int&, llvm::DINode::DIFlags, bool&, llvm::DITemplateParameterArray&, llvm::DISubprogram*)’
cargo:warning=       unwrapDIPtr<DISubprogram>(Decl));
cargo:warning=                                      ^
cargo:warning=In file included from ../rustllvm/rustllvm.h:53,
cargo:warning=                 from ../rustllvm/RustWrapper.cpp:1:
cargo:warning=/local_scratch/glaubitz/M680x0-llvm/include/llvm/IR/DIBuilder.h:663:5: note: candidate: ‘llvm::DISubprogram* llvm::DIBuilder::createFunction(llvm::DIScope*, llvm::StringRef, llvm::StringRef, llvm::DIFile*, unsigned int, llvm::DISubroutineType*, unsigned int, llvm::DINode::DIFlags, llvm::DISubprogram::DISPFlags, llvm::DITemplateParameterArray, llvm::DISubprogram*, llvm::DITypeArray)’
cargo:warning=     createFunction(DIScope *Scope, StringRef Name, StringRef LinkageName,
cargo:warning=     ^~~~~~~~~~~~~~
cargo:warning=/local_scratch/glaubitz/M680x0-llvm/include/llvm/IR/DIBuilder.h:663:5: note:   candidate expects 12 arguments, 13 provided
cargo:warning=../rustllvm/RustWrapper.cpp: In function ‘void LLVMRustUnpackOptimizationDiagnostic(LLVMDiagnosticInfoRef, RustStringRef, LLVMOpaqueValue**, unsigned int*, unsigned int*, RustStringRef, RustStringRef)’:
cargo:warning=../rustllvm/RustWrapper.cpp:923:23: error: ‘class llvm::DiagnosticLocation’ has no member named ‘getFilename’; did you mean ‘getLine’?
cargo:warning=     FilenameOS << loc.getFilename();
cargo:warning=                       ^~~~~~~~~~~
cargo:warning=                       getLine

Are there already patches to update rustllvm for the new LLVM version?

Edit: Ah, it's actually part of this PR ;). Will give it a try.

@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 Jan 25, 2019
@@ -1813,7 +1813,7 @@ name = "rand_chacha"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"rand_core 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rand_core 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would upgrading dlmalloc downgrade rand_core?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of those rand_core versions are already in the dependency tree, and rand_chacha requires rand_core >= 0.2, < 0.4. I guess it's just a quirk of how cargo resolved this when making the other change. But rand_core 0.2.2 just re-exports from 0.3 anyway, so it doesn't really matter.

cuviper and others added 8 commits January 25, 2019 15:39
The new git submodule src/llvm-project is a monorepo replacing src/llvm
and src/tools/{clang,lld,lldb}.  This also serves as a rebase for these
projects to the new 8.x branch from trunk.

The src/llvm-emscripten fork is unchanged for now.
Remove usage of an old and removed wasm intrinsic
@cuviper
Copy link
Member Author

cuviper commented Jan 26, 2019

I rebased to avoid Cargo.lock conflicts.

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jan 26, 2019

📌 Commit 059ed4f has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 26, 2019

⌛ Testing commit 059ed4f with merge 9df043b...

bors added a commit that referenced this pull request Jan 26, 2019
Rebase to the llvm-project monorepo

The new git submodule src/llvm-project is a monorepo replacing src/llvm
and src/tools/{clang,lld,lldb}.  This also serves as a rebase for these
projects to the new 8.x branch from trunk.

The src/llvm-emscripten fork is unchanged for now.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Jan 26, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 9df043b to master...

@bors bors merged commit 059ed4f into rust-lang:master Jan 26, 2019
@alexcrichton
Copy link
Member

🎊

@nikic
Copy link
Contributor

nikic commented Feb 1, 2019

Perf results are in: https://perf.rust-lang.org/compare.html?start=37d51aa8f3bca674a50eb7c6204deed6fb4dff80&end=9df043b543bb9bc3e50bc243811c58d52a3aefea&stat=instructions:u

Looks like mostly minor improvements for simple crates / incremental. Only one regression in kekkac-opt that might be worth taking a glance at.

@cuviper cuviper deleted the llvm-monorepo branch March 27, 2019 22:19
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
Fix rust-lldb wrapper scripts.

Currently the `rust-lldb` wrapper provided by Rust project is broken. The error messages it produces on launch are as follows:
```
warning: ignoring unknown option: --one-line-before-file=command script import "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/etc/lldb_rust_formatters.py"
warning: ignoring unknown option: --one-line-before-file=type summary add --no-value --python-function lldb_rust_formatters.print_val -x ".*" --category Rust
warning: ignoring unknown option: --one-line-before-file=type category enable Rust
(lldb) target create "target/debug/nagare"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/python2.7/site-packages/lldb/__init__.py", line 1481, in <module>
    class SBAddress(object):
  File "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/python2.7/site-packages/lldb/__init__.py", line 1647, in SBAddress
    __swig_getmethods__["module"] = GetModule
NameError: name '__swig_getmethods__' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
...etc.
```

The errors stem from two regressions: one caused by an LLVM upgrade and one caused by unintended upgrade to SWIG 4.0 (SWIG is a wrapper generator that is used to generate Python bindings for LLVM and LLDB.)

(Edit: found the exact dates) The SWIG breakage happened because of a Homebrew version upgrade on `nightly-2019-05-01-x86_64-apple-darwin` and the LLVM breakage happened on `nightly-2019-01-27-x86_64-apple-darwin` (likely to have been caused by rust-lang#57675 ).

The fix is to update the LLVM parameter syntax and to "downgrade" to SWIG 3.0.x. SWIG 3.0.x is not going to be supported by Homebrew forever, but should be good for now, until LLDB upgrades to  support SWIG 4.0.0. Here's some more info about Homebrew support: Homebrew/homebrew-core#39929 & Homebrew/homebrew-core#40882 I'm going to send a bug & fix to LLDB about SWIG 4.0.0 to get the situation fixed in the future.

It would be good to also backport this to beta, since it's such a small change, and will fix an obvious regression.
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
Fix rust-lldb wrapper scripts.

Currently the `rust-lldb` wrapper provided by Rust project is broken. The error messages it produces on launch are as follows:
```
warning: ignoring unknown option: --one-line-before-file=command script import "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/etc/lldb_rust_formatters.py"
warning: ignoring unknown option: --one-line-before-file=type summary add --no-value --python-function lldb_rust_formatters.print_val -x ".*" --category Rust
warning: ignoring unknown option: --one-line-before-file=type category enable Rust
(lldb) target create "target/debug/nagare"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/python2.7/site-packages/lldb/__init__.py", line 1481, in <module>
    class SBAddress(object):
  File "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/python2.7/site-packages/lldb/__init__.py", line 1647, in SBAddress
    __swig_getmethods__["module"] = GetModule
NameError: name '__swig_getmethods__' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
...etc.
```

The errors stem from two regressions: one caused by an LLVM upgrade and one caused by unintended upgrade to SWIG 4.0 (SWIG is a wrapper generator that is used to generate Python bindings for LLVM and LLDB.)

(Edit: found the exact dates) The SWIG breakage happened because of a Homebrew version upgrade on `nightly-2019-05-01-x86_64-apple-darwin` and the LLVM breakage happened on `nightly-2019-01-27-x86_64-apple-darwin` (likely to have been caused by rust-lang#57675 ).

The fix is to update the LLVM parameter syntax and to "downgrade" to SWIG 3.0.x. SWIG 3.0.x is not going to be supported by Homebrew forever, but should be good for now, until LLDB upgrades to  support SWIG 4.0.0. Here's some more info about Homebrew support: Homebrew/homebrew-core#39929 & Homebrew/homebrew-core#40882 I'm going to send a bug & fix to LLDB about SWIG 4.0.0 to get the situation fixed in the future.

It would be good to also backport this to beta, since it's such a small change, and will fix an obvious regression.
bors added a commit that referenced this pull request Jun 20, 2019
Fix rust-lldb wrapper scripts.

Currently the `rust-lldb` wrapper provided by Rust project is broken. The error messages it produces on launch are as follows:
```
warning: ignoring unknown option: --one-line-before-file=command script import "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/etc/lldb_rust_formatters.py"
warning: ignoring unknown option: --one-line-before-file=type summary add --no-value --python-function lldb_rust_formatters.print_val -x ".*" --category Rust
warning: ignoring unknown option: --one-line-before-file=type category enable Rust
(lldb) target create "target/debug/nagare"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/python2.7/site-packages/lldb/__init__.py", line 1481, in <module>
    class SBAddress(object):
  File "/Users/kon/.rustup/toolchains/nightly-2019-05-02-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/python2.7/site-packages/lldb/__init__.py", line 1647, in SBAddress
    __swig_getmethods__["module"] = GetModule
NameError: name '__swig_getmethods__' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
...etc.
```

The errors stem from two regressions: one caused by an LLVM upgrade and one caused by unintended upgrade to SWIG 4.0 (SWIG is a wrapper generator that is used to generate Python bindings for LLVM and LLDB.)

(Edit: found the exact dates) The SWIG breakage happened because of a Homebrew version upgrade on `nightly-2019-05-01-x86_64-apple-darwin` and the LLVM breakage happened on `nightly-2019-01-27-x86_64-apple-darwin` (likely to have been caused by #57675 ).

The fix is to update the LLVM parameter syntax and to "downgrade" to SWIG 3.0.x. SWIG 3.0.x is not going to be supported by Homebrew forever, but should be good for now, until LLDB upgrades to  support SWIG 4.0.0. Here's some more info about Homebrew support: Homebrew/homebrew-core#39929 & Homebrew/homebrew-core#40882 I'm going to send a bug & fix to LLDB about SWIG 4.0.0 to get the situation fixed in the future.

It would be good to also backport this to beta, since it's such a small change, and will fix an obvious regression.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.