-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Backports for 1.1.0-rc2 #30607
Backports for 1.1.0-rc2 #30607
Conversation
I've marked #30599 for backporting, which should fix the macOS CI. @staticfloat, it looks like we'll also need to backport whatever it is you did that fixed AppVeyor. |
So probably 8b189ec then |
(cherry picked from commit a5a5882)
This corresponds to macOS 10.12 Sierra. XCode 8 covers El Capitan and Sierra, so if Travis is giving us XCode 8.x for x < 3, we're on El Cap. Homebrew supports only three versions of macOS at a time, which means that El Cap (10.11) is no longer support. This is likely why our Mac builds are trying to build GCC from source; a bottle might not be available. (cherry picked from commit 862fe08)
PR #28478 moved the computation of the use counts before the finish call. to fix #28444. However, the early parts of the finish call fixes up phi node arguments, which fail to get counted if we look at use counts before that fixup is performed. This causes #30594 where the only non-trivial use is on the backedge of the phi and would thus incorrectly fail to get accounted for. Fix that by taking the use count after phi fixup but before dce. (cherry picked from commit f8f2045)
These changes cut some method dependency edges that tend to lead to invalidations when loading packages. (cherry picked from commit 13fc4c3)
SROA was accidentally treating a pending node as old and thus getting the wrong type when querying the predecessor. As a result it thought one of the paths was unreachable causing undefined data to be introduced on that path (generally the `1.0` that happened to already be in register). Fix #29983 (cherry picked from commit da0179c)
I don't have concrete tests for these, but it looks like they all need the `is_old` predicate for what they're doing, so switch those over also while we're at it. (cherry picked from commit 34f7a4a)
@nanosoldier |
@ararslan could you kick nanosoldier? |
The appveyor failure is bizarre; Line 96 in 346199d
That line hasn't changed in 3 years. Could |
Also why are we even using |
@nanosoldier |
This comment has been minimized.
This comment has been minimized.
We could try, but this bug has been fixed since (at least) v0.1alpha2.5 over 7 years ago, so it would just be an empty file. (OpenMathLib/OpenBLAS@0d76196) |
@nanosoldier |
I tried to just remove it. |
@@ -92,10 +92,6 @@ endif | |||
# Do not overwrite the "-j" flag | |||
OPENBLAS_BUILD_OPTS += MAKE_NB_JOBS=0 | |||
|
|||
$(BUILDDIR)/$(OPENBLAS_SRC_DIR)/build-configured: $(BUILDDIR)/$(OPENBLAS_SRC_DIR)/source-extracted | |||
perl -i -ple 's/^\s*(EXTRALIB\s*\+=\s*-lSystemStubs)\s*$$/# $$1/g' $(dir $<)/Makefile.system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you still need to create the build-configured
file though; so just remove the perl
invocation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, thanks.
6ee9441
to
17a5e1e
Compare
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
AV seems to build OpenBLAS from scratch now and times out during test on 32 bit but fails on 64 bit with:
Did I do something bad in the last commit? Help plz. |
PkgEval shows no regressions since RC1. Note that it was run prior to the most recent Makefile changes. |
I'm not sure how, but it looks like you only got part of 87c18d8. You need the rest of the changes as well. |
Well, that wasn't marked with backport label and it wasn't the commit in #30607 (comment) so it wasn't backported. I'll add that as well then. But why do we need so many build backports to this branch? Did something change on AV's end? |
Sorry about that.
Yes, the compiler versions changed, but also it's an improvement that reduces compile/test times, so it seemed like a good thing to include. |
(cherry picked from commit 2a2ba37)
* Auto-detect binarybuilder triplet * Add OpenBLAS BinaryBuilder installation scaffolding Also make it easier to add more BB-cached versions of dependencies in the future * Enable `fixup-libgfortran.sh` to directly ask `$FC` for paths * Tell Appveyor and Travis to use BinaryBuilder OpenBLAS Also allow the build system to auto-guess the triplet (cherry picked from commit 87c18d8)
* Fix `bb-install` naming conventions, add hashes * Set `DEP_LIBS` to include `openblas` on Appveyor * When guessing BB libc, default to `glibc` on Linux * Fix bb-install bash parsing failure Quote to avoid bash freaking out from spurious `)` from compilers that have fancy version strings such as `(Red Hat 7.3.1-5)`, which gets `lastword`'ed down to `7.3.1-5)`. * Add `contrib/refresh_bb_tarballs.sh` to aid in batch-grabbing BB hashes (cherry picked from commit 8b189ec)
17a5e1e
to
05a8951
Compare
Backported those two but something is still wrong :(
|
I have no idea what Appveyor is smoking there. That doesn't make any sense to me, especially as we can see other processes happily going and entering that directory. |
Ok, this happens on master as well. Doesn't stop the tests from running apparently. |
Backports for 1.1.0-rc2
Backported PRs:
_apply
match the implementation more closely #30483 - make inference of_apply
match the implementation more closelyreturn_type
#30470 - use type inference world inreturn_type
bb-install
naming conventions, add hashes #30535 - Fixbb-install
naming conventions, add hashesNeed manual backport:
Non-merged PRs with backport label: