rustc_codegen_llvm: begin generalizing over backend values. #52987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.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 |
|
The job Click to expand the log.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 |
That's... stage0 (beta) segfaulting, quite unexpected. cc @alexcrichton @rust-lang/compiler |
|
What other |
|
We're expecting to implement a Cranelift backend soon so we'll have Cranelift IR values to deal with :) |
|
Travis log seems to indicate the segfault is in LLVM: |
|
Oh this might be an old bug from LLVM, I tried building locally and had no issues. |
|
I got the segfault too, with |
|
@denismerigoux Hmm, do you have LLVM assertions disabled in |
|
@eddyb I do have LLVM assertions enabled in |
|
@eddyb @irinagpopa I rebased the current |
|
The master branch has since upgraded to LLVM 7, so perhaps this was a bug in LLVM that was fixed? |
|
Sorry for the false hope, I didn't recompile properly the time it went through without the segfault. I rebuilt from scratch and I confirm that the segfault still exists even after rebasing on top of the current |
|
I managed to get a failed assertion during a failed build : |
|
The implication of |
|
I managed to get a backtrace of the failed assertion using |
|
☔ The latest upstream changes (presumably #51131) made this pull request unmergeable. Please resolve the merge conflicts. |
pietroalbini
added
S-waiting-on-author
|
Starting from @denismerigoux and @irinagpopa's attempts, I was able to reduce the bug: #53912. |
bors
added a commit
that referenced
this pull request
Sep 7, 2018
rustc_codegen_llvm: traitification of LLVM-specific CodegenCx and Builder methods This PR is the continuation of #52461 in the grand plan of #45274 to allow for multiple codegen backends. A first attempt at this was #52987 but since @irinagpopa is no longer working on it I'm taking ownership of the PR. The changes are refactoring only and do not affect the logic of the code. Performance should not be impacted since all parametrization is done with generics (no trait objects). The `librustc_codegen_llvm` crate now contains a new folder `interfaces` that describes with traits part of how the compiler interfaces with LLVM during codegen. `CodegenCx` and `Builder` implement those traits. Many things are still missing. All the calls to LLVM are not yet under a trait, and later LLVM-agnostic code should be parametrized.
|
Ping from triage, @irinagpopa! It looks like there's been some progress on tracking down the cause of the Travis failure. |
|
@denismerigoux has taken over this patch series and already has a PR that includes this commit and a workaround for the LLVM bug. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #45274.