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

rustc: Update LLVM #26025

Merged
merged 1 commit into from
Jun 17, 2015
Merged

rustc: Update LLVM #26025

merged 1 commit into from
Jun 17, 2015

Conversation

alexcrichton
Copy link
Member

This commit updates the LLVM submodule in use to the current HEAD of the LLVM
repository. This is primarily being done to start picking up unwinding support
for MSVC, which is currently unimplemented in the revision of LLVM we are using.
Along the way a few changes had to be made:

  • As usual, lots of C++ debuginfo bindings in LLVM changed, so there were some
    significant changes to our RustWrapper.cpp
  • As usual, some pass management changed in LLVM, so clang was re-scrutinized to
    ensure that we're doing the same thing as clang.
  • Some optimization options are now passed directly into the
    PassManagerBuilder instead of through CLI switches to LLVM.
  • The NoFramePointerElim option was removed from LLVM, favoring instead the
    no-frame-pointer-elim function attribute instead.
  • The LoopVectorize option of the LLVM optimization passes has been disabled
    as it causes a divide-by-zero exception to happen in LLVM for zero-sized
    types. This is reported as https://llvm.org/bugs/show_bug.cgi?id=23763

Additionally, LLVM has picked up some new optimizations which required fixing an
existing soundness hole in the IR we generate. It appears that the current LLVM
we use does not expose this hole. When an enum is moved, the previous slot in
memory is overwritten with a bit pattern corresponding to "dropped". When the
drop glue for this slot is run, however, the switch on the discriminant can
often start executing the unreachable block of the switch due to the
discriminant now being outside the normal range. This was patched over locally
for now by having the unreachable block just change to a ret void.

@rust-highfive
Copy link
Collaborator

r? @pcwalton

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

@alexcrichton
Copy link
Member Author

r? @brson

cc @rust-lang/compiler - something to be aware of at least, and I may have updated something horribly incorrectly

@pnkfelix
Copy link
Member

pnkfelix commented Jun 5, 2015

@eddyb notes on a comment from #23535 that we may want to restrict the ret void change so that it is only injected on the switch injected in the generated drop-glue.

@sanxiyn
Copy link
Member

sanxiyn commented Jun 5, 2015

Apparently, LLVM PR23763 is fixed now.

@alexcrichton
Copy link
Member Author

@pnkfelix I think we're covered on that front because the function iter_structural_ty is only called from generating the drop glue.

@sanxiyn it has indeed! I'm testing out pulling in that revision and running tests now

@eddyb
Copy link
Member

eddyb commented Jun 5, 2015

@alexcrichton oh, of course, I forgot that. Maybe that machinery should be moved out of trans::base then (in the future).

@alexcrichton
Copy link
Member Author

Looks like rust-lang/llvm@47dfcb7 did indeed fix the problems I was seeing! Updated the LLVM submodule.

unwrap(PMB)->SLPVectorize = SLPVectorize;
unwrap(PMB)->OptLevel = OptLevel;
unwrap(PMB)->LoopVectorize = LoopVectorize;
#endif
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the PMB opt level is set here and not through llvm::LLVMPassManagerBuilderSetOptLevel it appears that for LLVM < 7 we don't set the PMB opt level. Is that true, and is that bad?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh this reminded me that I meant to recompile these files against 3.5 and 3.6, and now having done so it looks like only MergeFunctions wasn't available in 3.5 but everything else is now set on all versions.

@brson
Copy link
Contributor

brson commented Jun 5, 2015

r=me once you've answered my q's.

This was a tough upgrade. Good work.

@alexcrichton
Copy link
Member Author

@bors: r=brson 0e0cc9e

@bors
Copy link
Contributor

bors commented Jun 5, 2015

⌛ Testing commit 0e0cc9e with merge 2146851...

bors added a commit that referenced this pull request Jun 5, 2015
This commit updates the LLVM submodule in use to the current HEAD of the LLVM
repository. This is primarily being done to start picking up unwinding support
for MSVC, which is currently unimplemented in the revision of LLVM we are using.
Along the way a few changes had to be made:

* As usual, lots of C++ debuginfo bindings in LLVM changed, so there were some
  significant changes to our RustWrapper.cpp
* As usual, some pass management changed in LLVM, so clang was re-scrutinized to
  ensure that we're doing the same thing as clang.
* Some optimization options are now passed directly into the
  `PassManagerBuilder` instead of through CLI switches to LLVM.
* The `NoFramePointerElim` option was removed from LLVM, favoring instead the
  `no-frame-pointer-elim` function attribute instead.
* The `LoopVectorize` option of the LLVM optimization passes has been disabled
  as it causes a divide-by-zero exception to happen in LLVM for zero-sized
  types. This is reported as https://llvm.org/bugs/show_bug.cgi?id=23763

Additionally, LLVM has picked up some new optimizations which required fixing an
existing soundness hole in the IR we generate. It appears that the current LLVM
we use does not expose this hole. When an enum is moved, the previous slot in
memory is overwritten with a bit pattern corresponding to "dropped". When the
drop glue for this slot is run, however, the switch on the discriminant can
often start executing the `unreachable` block of the switch due to the
discriminant now being outside the normal range. This was patched over locally
for now by having the `unreachable` block just change to a `ret void`.
@bors
Copy link
Contributor

bors commented Jun 6, 2015

💔 Test failed - auto-win-gnu-32-nopt-t

@alexcrichton
Copy link
Member Author

@bors: r=brson 89cb355

@bors
Copy link
Contributor

bors commented Jun 6, 2015

⌛ Testing commit 89cb355 with merge 13e5306...

@bors
Copy link
Contributor

bors commented Jun 6, 2015

💔 Test failed - auto-mac-64-nopt-t

@alexcrichton
Copy link
Member Author

This is now blocked on https://llvm.org/bugs/show_bug.cgi?id=23779

@bors
Copy link
Contributor

bors commented Jun 9, 2015

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

@alexcrichton
Copy link
Member Author

Ok, the LLVM bug has not been fixed yet, and I had some time to do some more investigation. The end result was adding this extra commit to the LLVM branch that we're using. Still running tests locally, but I suspect it will fix that segfault at least.

re-r? @brson

@bors
Copy link
Contributor

bors commented Jun 17, 2015

⌛ Testing commit 6583dda with merge 252855f...

@bors
Copy link
Contributor

bors commented Jun 17, 2015

💔 Test failed - auto-mac-64-nopt-t

@retep998
Copy link
Member

Do you think you could fix this error before trying again?

In file included from /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.cpp:10:
In file included from /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h:20:
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/src/llvm/include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h:126:66: error: only virtual member functions can be marked 'override'
                                 bool ExportedSymbolsOnly) const override {
                                                                 ^~~~~~~~
1 error generated.
make[4]: *** [/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-nopt-t/build/obj/x86_64-apple-darwin/llvm/lib/ExecutionEngine/Orc/Release/OrcMCJITReplacement.o] Error 1
make[3]: *** [Orc/.makeall] Error 2
make[2]: *** [ExecutionEngine/.makeall] Error 2
make[2]: *** Waiting for unfinished jobs....

@alexcrichton
Copy link
Member Author

@bors: r=brson 547418d

@alexcrichton
Copy link
Member Author

@bors: r=brson 78ac14c

@bors
Copy link
Contributor

bors commented Jun 17, 2015

⌛ Testing commit 78ac14c with merge 87f60f5...

@bluss bluss mentioned this pull request Jun 17, 2015
29 tasks
@bors
Copy link
Contributor

bors commented Jun 17, 2015

💔 Test failed - auto-mac-32-opt

This commit updates the LLVM submodule in use to the current HEAD of the LLVM
repository. This is primarily being done to start picking up unwinding support
for MSVC, which is currently unimplemented in the revision of LLVM we are using.
Along the way a few changes had to be made:

* As usual, lots of C++ debuginfo bindings in LLVM changed, so there were some
  significant changes to our RustWrapper.cpp
* As usual, some pass management changed in LLVM, so clang was re-scrutinized to
  ensure that we're doing the same thing as clang.
* Some optimization options are now passed directly into the
  `PassManagerBuilder` instead of through CLI switches to LLVM.
* The `NoFramePointerElim` option was removed from LLVM, favoring instead the
  `no-frame-pointer-elim` function attribute instead.

Additionally, LLVM has picked up some new optimizations which required fixing an
existing soundness hole in the IR we generate. It appears that the current LLVM
we use does not expose this hole. When an enum is moved, the previous slot in
memory is overwritten with a bit pattern corresponding to "dropped". When the
drop glue for this slot is run, however, the switch on the discriminant can
often start executing the `unreachable` block of the switch due to the
discriminant now being outside the normal range. This was patched over locally
for now by having the `unreachable` block just change to a `ret void`.
@alexcrichton
Copy link
Member Author

@bors: r=brson f9d4149

@bors
Copy link
Contributor

bors commented Jun 17, 2015

⌛ Testing commit f9d4149 with merge aa00f2e...

bors added a commit that referenced this pull request Jun 17, 2015
This commit updates the LLVM submodule in use to the current HEAD of the LLVM
repository. This is primarily being done to start picking up unwinding support
for MSVC, which is currently unimplemented in the revision of LLVM we are using.
Along the way a few changes had to be made:

* As usual, lots of C++ debuginfo bindings in LLVM changed, so there were some
  significant changes to our RustWrapper.cpp
* As usual, some pass management changed in LLVM, so clang was re-scrutinized to
  ensure that we're doing the same thing as clang.
* Some optimization options are now passed directly into the
  `PassManagerBuilder` instead of through CLI switches to LLVM.
* The `NoFramePointerElim` option was removed from LLVM, favoring instead the
  `no-frame-pointer-elim` function attribute instead.
* The `LoopVectorize` option of the LLVM optimization passes has been disabled
  as it causes a divide-by-zero exception to happen in LLVM for zero-sized
  types. This is reported as https://llvm.org/bugs/show_bug.cgi?id=23763

Additionally, LLVM has picked up some new optimizations which required fixing an
existing soundness hole in the IR we generate. It appears that the current LLVM
we use does not expose this hole. When an enum is moved, the previous slot in
memory is overwritten with a bit pattern corresponding to "dropped". When the
drop glue for this slot is run, however, the switch on the discriminant can
often start executing the `unreachable` block of the switch due to the
discriminant now being outside the normal range. This was patched over locally
for now by having the `unreachable` block just change to a `ret void`.
@bors bors merged commit f9d4149 into rust-lang:master Jun 17, 2015
@alexcrichton alexcrichton deleted the update-llvm branch June 17, 2015 16:24
@KindDragon
Copy link

Congratulations! 👍 😄

@frewsxcv
Copy link
Member

This caused a regression related to LLVM passes. I fixed it in #31176

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jan 25, 2016
LLVM was upgraded to a new version in this commit:

rust-lang@f9d4149

which was part of this pull request:

rust-lang#26025

Consider the following two lines from that commit:

rust-lang@f9d4149#diff-a3b24dbe2ea7c1981f9ac79f9745f40aL462

rust-lang@f9d4149#diff-a3b24dbe2ea7c1981f9ac79f9745f40aL469

The purpose of these lines is to register LLVM passes. Prior to the that
commit, the passes being handled were assumed to be ModulePasses (a
specific type of LLVM pass) since they were being added to a ModulePass
manager. After that commit, both lines were refactored (presumably in an
attempt to DRY out the code), but the ModulePasses were changed to be
registered to a FunctionPass manager. This change resulted in
ModulePasses being run, but a Function object was being passed as a
parameter to the pass instead of a Module, which resulted in
segmentation faults.

In this commit, I changed relevant sections of the code to check the
type of the passes being added and register them to the appropriate pass
manager.

Closes rust-lang#31067
bors added a commit that referenced this pull request Jan 25, 2016
Register LLVM passes with the correct LLVM pass manager.

LLVM was upgraded to a new version in this commit:

f9d4149

which was part of this pull request:

#26025

Consider the following two lines from that commit:

f9d4149#diff-a3b24dbe2ea7c1981f9ac79f9745f40aL462

f9d4149#diff-a3b24dbe2ea7c1981f9ac79f9745f40aL469

The purpose of these lines is to register LLVM passes. Prior to the that
commit, the passes being handled were assumed to be ModulePasses (a
specific type of LLVM pass) since they were being added to a ModulePass
manager. After that commit, both lines were refactored (presumably in an
attempt to DRY out the code), but the ModulePasses were changed to be
registered to a FunctionPass manager. This change resulted in
ModulePasses being run, but a Function object was being passed as a
parameter to the pass instead of a Module, which resulted in
segmentation faults.

In this commit, I changed relevant sections of the code to check the
type of the passes being added and register them to the appropriate pass
manager.

Closes #31067
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.