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.

Sign up for GitHub

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

Jump to bottom

Upgrade to LLVM 14.0.2 #45195

Merged
merged 5 commits into from
Jun 14, 2022
Merged

Upgrade to LLVM 14.0.2 #45195

merged 5 commits into from
Jun 14, 2022

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented May 5, 2022 •
edited
Loading

TODO:

  • analyzegc
  • llvmpasses
  • Win64 Build
  • ASAN
  • 8X performance foldl
  • Mac/Codegen failure
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-macmini-x64-5.0/build/default-macmini-x64-5-0/julialang/julia-master/julia-538dba5996/share/julia/test/compiler/codegen.jl:272
  Expression: was_gced

@vchuravy vchuravy requested review from vtjnash, staticfloat, pchintalapudi, giordano and tkf and removed request for vtjnash May 5, 2022 15:00
pchintalapudi
pchintalapudi reviewed May 5, 2022
View reviewed changes
Makefile Show resolved Hide resolved
@vchuravy vchuravy added this to the 1.9 milestone May 5, 2022
vchuravy
vchuravy commented May 5, 2022
View reviewed changes
deps/checksums/llvm Outdated Show resolved Hide resolved
@pchintalapudi
Copy link
Member

I think the current build error can be fixed by moving #define DEBUG_TYPE lower down in our optimization files (probably DomTreeBuilder defines and undefs its own DEBUG_TYPE).

@vchuravy vchuravy force-pushed the vc/llvm_14.0.2 branch 2 times, most recently from 3d35596 to b3febcb Compare May 6, 2022 19:30
@vchuravy
Copy link
Member Author

vchuravy commented May 6, 2022

Win64 looks like a fun one:

 cd /cygdrive/c/buildbot/worker/package_win64/build/base && /cygdrive/c/buildbot/worker/package_win64/build/usr/bin/julia.exe -C "generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)" --output-ji `cygpath -w /cygdrive/c/buildbot/worker/package_win64/build/usr/lib/julia/corecompiler.ji`.tmp --startup-file=no --warn-overwrite=yes -g0 -O0 compiler/compiler.jl
Assertion failed: inserted, file /cygdrive/c/buildbot/worker/package_win64/build/src/jitlayers.cpp, line 763

@kshyatt kshyatt added the external dependencies Involves LLVM, OpenBLAS, or other linked libraries label May 7, 2022
@vchuravy
Copy link
Member Author

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@vchuravy
Copy link
Member Author

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@pchintalapudi
Copy link
Member

foldl seems to be the biggest regression here (~8X), and some of the summation benchmarks seem to have improved for floats.

@gbaraldi
Copy link
Member

That big of change sounds like a failure to SIMD

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@vchuravy
Copy link
Member Author

The aarch64 failure is known:

      From worker 4:	julia: /workspace/srcdir/llvm-project/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:507: void llvm::RuntimeDyldELF::resolveAArch64Relocation(const llvm::SectionEntry&, uint64_t, uint64_t, uint32_t, int64_t): Assertion `isInt<33>(Result) && "overflow check failed for relocation"' failed.
      From worker 4:	
      From worker 4:	signal (6): Aborted

It's the reason we switched to JITLink for aarch64-darwin. cc: @lhames @weliveindetail I will happily offer support if the GSoC student wants use Julia as a stress-test for JITLink on Elf

@vchuravy
Copy link
Member Author 

compiler/codegen                          (3) |         failed at 2022-05-21T21:12:00.891
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-macmini-x64-5.0/build/default-macmini-x64-5-0/julialang/julia-master/julia-538dba5996/share/julia/test/compiler/codegen.jl:272
  Expression: was_gced

on aarch64-darwin and

signal (11): Segmentation fault
in expression starting at none:0
jl_is_submodule at /workspace/srcdir/src/module.c:901
module_in_worklist at /workspace/srcdir/src/dump.c:182 [inlined]
jl_collect_backedges at /workspace/srcdir/src/dump.c:1233 [inlined]
ijl_save_incremental at /workspace/srcdir/src/dump.c:2682
jl_write_compiler_output at /workspace/srcdir/src/precompile.c:65
ijl_atexit_hook at /workspace/srcdir/src/init.c:204
jl_repl_entrypoint at /workspace/srcdir/src/jlapi.c:711
main at /workspace/srcdir/cli/loader_exe.c:59
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x400808)
Allocations: 11073626 (Pool: 11067760; Big: 5866); GC: 15
ERROR: LoadError: Failed to precompile Colors [5ae59095-9a9b-59fe-a467-6f913c188581] to /home/pkgeval/.julia/compiled/v1.9/Colors/jl_nAutxB.

from PkgEval seem to be the biggest issues. Both are GC related.

@vchuravy
Copy link
Member Author

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@pchintalapudi
Copy link
Member

foldl regression is probably this: https://reviews.llvm.org/D119854. We probably need to take a closer look at which simplifycfg options we actually want at various places in the pipeline.

@vchuravy vchuravy mentioned this pull request May 24, 2022
Closed
@vchuravy
Copy link
Member Author

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@vchuravy
Copy link
Member Author

@vtjnash I am stuck on the Windows test somehow a linux path creeps into the backtrace.

@vtjnash vtjnash mentioned this pull request Jun 10, 2022
Merged
vchuravy and others added 5 commits June 11, 2022 20:53
@vchuravy
Upgrade to LLVM 14.0.2
bb9d7f6
@vchuravy
Replace debuginfo GVs for Win64 with assembly
dac857e 
https://reviews.llvm.org/D100944 introduces will split sections if the
metadata does not match. The GlobalVariables are `RW_` and the `.text`
section is `R_X`. Currently there is no facility to actually mark the
GV as `R_X` and we only need to write them during code-emission before
we flip the permissions.
@vchuravy
Help the static analyzer
805cf87
@vchuravy
fix llvmpasses for 14
5fb9b6f
@pchintalapudi @vchuravy
Adjust simplifycfg options
1c5858b
@vchuravy
Copy link
Member Author

@nanosoldier runtests(ALL, vs = ":master")

@vchuravy
Copy link
Member Author

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@KristofferC
Copy link
Sponsor Member

Maybe want to run PkgEval with assertions for PRs like this?

@vchuravy
Copy link
Member Author

Maybe want to run PkgEval with assertions for PRs like this?

That would be a great option to have!

@vchuravy
Copy link
Member Author

@DilumAluthge @tkf and I chatted about ASAN and we decided that for now we should make it "allowed to fail" best to keep the "can we build" bit though.

@DilumAluthge
Copy link
Member

Maybe want to run PkgEval with assertions for PRs like this?

That would be a great option to have!

I believe we already have this option. Control-F https://github.com/JuliaCI/Nanosoldier.jl/blob/master/README.md for "assert".

@vchuravy
Copy link
Member Author

@nanosoldier runtests(ALL, vs = "%self", buildflags=["LLVM_ASSERTIONS=1", "FORCE_ASSERTIONS=1"])

@tkf tkf mentioned this pull request Jun 13, 2022
Closed
@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@tkf tkf mentioned this pull request Jun 13, 2022
Merged
@vchuravy vchuravy merged commit 6c97a48 into master Jun 14, 2022
@vchuravy vchuravy deleted the vc/llvm_14.0.2 branch June 14, 2022 11:35
@KristofferC
Copy link
Sponsor Member

Feels like this should give some comment about all the assertion failures in the PkgEval report.

@maleadt
Copy link
Member

maleadt commented Jun 14, 2022

It probably also should have been a comparison to master with assertions, so that any assertion reported would have been a new one.

@vchuravy vchuravy mentioned this pull request Jun 14, 2022
Merged
@vchuravy
Copy link
Member Author

Feels like this should give some comment about all the assertion failures in the PkgEval report.

... I am blind. We really need to improve the formatting (and order of the report). I looked at the report on my phone and mentally lumped the process aborted and segmentation fault reports under "Networking related" since the heading got completely lost. Sorry about that.

@fingolfin fingolfin mentioned this pull request Jun 14, 2022
Closed
@gbaraldi
Copy link
Member

We should probably run PkgEval with assertions periodically to catch some assertion specific errors earlier.

@vchuravy vchuravy mentioned this pull request Jun 14, 2022
Merged
@giordano giordano mentioned this pull request Jul 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchintalapudi pchintalapudi pchintalapudi left review comments

@vtjnash vtjnash vtjnash approved these changes

@staticfloat staticfloat Awaiting requested review from staticfloat

@giordano giordano Awaiting requested review from giordano

@tkf tkf Awaiting requested review from tkf

Assignees
No one assigned
Labels
external dependencies Involves LLVM, OpenBLAS, or other linked libraries
Projects
None yet
Milestone
1.9
Development

Successfully merging this pull request may close these issues.
10 participants
@vchuravy @pchintalapudi @nanosoldier @gbaraldi @giordano @KristofferC @DilumAluthge @maleadt @vtjnash @kshyatt