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

Prepare for LLVM 10 upgrade #67900

Merged
merged 18 commits into from
Jan 13, 2020
Merged

Prepare for LLVM 10 upgrade #67900

merged 18 commits into from
Jan 13, 2020

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jan 5, 2020

Split off from #67759, this just adds the necessary compatibility bits and updates codegen tests, without performing the actual LLVM upgrade.

r? @alexcrichton

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2020
@nikic nikic mentioned this pull request Jan 5, 2020
8 tasks
Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

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

This PR removes debug info tests for LLVM 5/6, but this PR does not explicitly appear to remove any code that would otherwise break those tests (even though LLVM 6 is no longer supported AFAIK)?

Either way r=me once we flesh out the data layout string story.

@@ -156,6 +172,13 @@ pub unsafe fn create_module(
if llvm_util::get_major_version() < 9 {
target_data_layout = strip_function_ptr_alignment(target_data_layout);
}
if sess.target.target.arch == "x86" || sess.target.target.arch == "x86_64" {
Copy link
Member

Choose a reason for hiding this comment

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

We should do either one or the other, not both, if either one at all. i.e. we ought to pick whether our target data layouts strings are llvm10 (which IMO they should be) or something else. Then we don’t need code that can both "upgrade" and "downgrade" the string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's three parts here:

  • Data layout in our own target definitions: I agree that we should update those to use the LLVM 10 data layouts.
  • Downgrade support: If we use LLVM 10 data layout, then we must have downgrade support, otherwise older LLVM versions cannot be used anymore.
  • Upgrade support: Upgrade support is needed if we want to keep 3rd party target definitions (for x86 based targets) working. If we're okay with breaking these and have no stability guarantees in this area, then we can drop the upgrade support.

Copy link
Member

Choose a reason for hiding this comment

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

I don’t believe we have any stability guarantees for external target files. Going in that direction would necessitate removing at least some fields from target specs and data layout would be one of those fields -- we dont want to be on the hook for ensuring stability of LLVM's interfaces.

With that in mind I'd be happy with this if we only kept the downgrade code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've dropped the upgrade code and updated all the in-tree targets ... hopefully without typos ;)

@nikic
Copy link
Contributor Author

nikic commented Jan 6, 2020

This PR removes debug info tests for LLVM 5/6, but this PR does not explicitly appear to remove any code that would otherwise break those tests (even though LLVM 6 is no longer supported AFAIK)?

Right, these tests should have been dropped when LLVM 6 support was dropped (I can't find the relevant PR, because searching for LLVM in PRs is impossible...) The tests fail under LLVM 10 because LLVM 10 is not included in the ignore line, but it seemed to make little sense to extend the ignore if there aren't any non-ignored versions left anymore...

@alexcrichton
Copy link
Member

Oh wow that's a good number of changes! I feel the same way as @nagisa, so I defer to ...

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned alexcrichton Jan 6, 2020
@nagisa
Copy link
Member

nagisa commented Jan 8, 2020

@bors r+ rollup=never

@nagisa nagisa closed this Jan 8, 2020
@bors
Copy link
Contributor

bors commented Jan 8, 2020

📌 Commit c9e996f has been approved by nagisa

@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 8, 2020
@nagisa nagisa reopened this Jan 8, 2020
@bors
Copy link
Contributor

bors commented Jan 13, 2020

⌛ Testing commit c9e996f with merge e82febc...

bors added a commit that referenced this pull request Jan 13, 2020
Prepare for LLVM 10 upgrade

Split off from #67759, this just adds the necessary compatibility bits and updates codegen tests, without performing the actual LLVM upgrade.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Jan 13, 2020

☀️ Test successful - checks-azure
Approved by: nagisa
Pushing e82febc to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 13, 2020
@bors bors merged commit c9e996f into rust-lang:master Jan 13, 2020
bors added a commit that referenced this pull request Feb 5, 2020
Update to LLVM 10

LLVM 10 is going to be branched soon, so it's a good time to start finding all those tasty new miscompiles and performance regressions ;)

Status:

 * Preparation split off into #67900.
 * Optimization regressions:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44419 => https://reviews.llvm.org/D72048 has landed.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44423 => https://reviews.llvm.org/D72060 has landed.
   * [ ] https://reviews.llvm.org/D72169 submitted.
   * [ ] https://bugs.llvm.org/show_bug.cgi?id=44461 reported. https://reviews.llvm.org/D72420 submitted, but unlikely eligible for LLVM 10.
 * Compile-time regressions:
   * [x] GlobalOpt regression identified. ~~fhahn proposed https://reviews.llvm.org/D72214.~~ fhahn has [reverted](llvm/llvm-project@192cce1) the patch.
   * [ ] Even with the revert, there are [large regressions](https://perf.rust-lang.org/compare.html?start=760ce94c69ca510d44087291c311296f6d9ccdf5&end=4e84f97d76e694bb9f59039f5bdeb6d8bca46d14).
 * Assertion failures:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44600 => https://reviews.llvm.org/D73135, https://reviews.llvm.org/D73854 and https://reviews.llvm.org/D73908 have landed and been cherry-picked to the 10.x branch.

r? @ghost
bors added a commit that referenced this pull request Feb 10, 2020
Update to LLVM 10

LLVM 10 is going to be branched soon, so it's a good time to start finding all those tasty new miscompiles and performance regressions ;)

Status:

 * Preparation split off into #67900.
 * Optimization regressions:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44419 => https://reviews.llvm.org/D72048 has landed.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44423 => https://reviews.llvm.org/D72060 has landed.
   * [ ] https://reviews.llvm.org/D72169 submitted.
   * [ ] https://bugs.llvm.org/show_bug.cgi?id=44461 reported. https://reviews.llvm.org/D72420 submitted, but unlikely eligible for LLVM 10.
 * Compile-time regressions:
   * [x] GlobalOpt regression identified. ~~fhahn proposed https://reviews.llvm.org/D72214.~~ fhahn has [reverted](llvm/llvm-project@192cce1) the patch.
   * [ ] Even with the revert, there are [large regressions](https://perf.rust-lang.org/compare.html?start=760ce94c69ca510d44087291c311296f6d9ccdf5&end=4e84f97d76e694bb9f59039f5bdeb6d8bca46d14).
 * Assertion failures / infinite loops:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44600 => https://reviews.llvm.org/D73135, https://reviews.llvm.org/D73854 and https://reviews.llvm.org/D73908 have landed and been cherry-picked to the 10.x branch.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44835 => https://reviews.llvm.org/D74278 has landed and been cherry-picked.

r? @ghost
bors added a commit that referenced this pull request Mar 3, 2020
Update to LLVM 10

LLVM 10 is going to be branched soon, so it's a good time to start finding all those tasty new miscompiles and performance regressions ;)

Status:

 * Preparation split off into #67900.
 * Optimization regressions:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44419 => https://reviews.llvm.org/D72048 has landed.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44423 => https://reviews.llvm.org/D72060 has landed.
   * [ ] https://reviews.llvm.org/D72169 submitted.
   * [ ] https://bugs.llvm.org/show_bug.cgi?id=44461 reported. https://reviews.llvm.org/D72420 submitted, but unlikely eligible for LLVM 10.
 * Compile-time regressions:
   * [x] GlobalOpt regression identified. ~~fhahn proposed https://reviews.llvm.org/D72214.~~ fhahn has [reverted](llvm/llvm-project@192cce1) the patch.
   * [ ] Even with the revert, there are [large regressions](https://perf.rust-lang.org/compare.html?start=760ce94c69ca510d44087291c311296f6d9ccdf5&end=4e84f97d76e694bb9f59039f5bdeb6d8bca46d14).
 * Assertion failures / infinite loops:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44600 => https://reviews.llvm.org/D73135, https://reviews.llvm.org/D73854 and https://reviews.llvm.org/D73908 have landed and been cherry-picked to the 10.x branch.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44835 => https://reviews.llvm.org/D74278 has landed and been cherry-picked.

r? @ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2020
Update to LLVM 10

LLVM 10 is going to be branched soon, so it's a good time to start finding all those tasty new miscompiles and performance regressions ;)

Status:

 * Preparation split off into rust-lang#67900.
 * Optimization regressions:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44419 => https://reviews.llvm.org/D72048 has landed.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44423 => https://reviews.llvm.org/D72060 has landed.
   * [x] https://reviews.llvm.org/D72169 submitted.
   * [ ] https://bugs.llvm.org/show_bug.cgi?id=44461 reported. https://reviews.llvm.org/D72420 submitted, but unlikely eligible for LLVM 10.
 * Compile-time regressions:
   * [x] GlobalOpt regression identified. ~~fhahn proposed https://reviews.llvm.org/D72214.~~ fhahn has [reverted](llvm/llvm-project@192cce1) the patch.
   * [ ] Even with the revert, there are [large regressions](https://perf.rust-lang.org/compare.html?start=760ce94c69ca510d44087291c311296f6d9ccdf5&end=4e84f97d76e694bb9f59039f5bdeb6d8bca46d14).
 * Assertion failures / infinite loops:
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44600 => https://reviews.llvm.org/D73135, https://reviews.llvm.org/D73854 and https://reviews.llvm.org/D73908 have landed and been cherry-picked to the 10.x branch.
   * [x] https://bugs.llvm.org/show_bug.cgi?id=44835 => https://reviews.llvm.org/D74278 has landed and been cherry-picked.
RalfJung added a commit to RalfJung/rust that referenced this pull request May 22, 2020
…es, r=Amanieu

Enable ARM TME (Transactional Memory Extensions)

Enables ARM TME coming up with LLVM 10. Related ARM TME intrinsics are included by the merge of rust-lang#67900.

Enables: rust-lang/stdarch#855
jclulow added a commit to jclulow/rust that referenced this pull request May 28, 2020
In a recent change, 8b19922,
adjustments were made to the data layout we pass to LLVM.
Unfortunately, the illumos target was missed in this change.
See also: rust-lang#67900
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 29, 2020
update data layout for illumos x86

In a recent change, 8b19922,
adjustments were made to the data layout we pass to LLVM.
Unfortunately, the illumos target was missed in this change.
See also: rust-lang#67900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

5 participants