Add ScalarEvolutionExpander patch.

[blakejohnson]

Adds @carnaval and @vtjnash's ScalarEvolutionExpander patch, which fixes #14704.

[blakejohnson]

I'd don't have enough Makefile-fu to understand why this line exists. So, I didn't know if I should update the target to $(LLVM_SRC_DIR)/llvm-3.7.1_3.patch-applied.

[yuyichao]

IIUC this extra dependency is needed since the two patch updates the same file. Should be fine if the third patch is independent with the other two.

[nalimilan]

If I understand the code correctly, this ensures the patches are applied in the correct order. This doesn't make any difference if they don't touch the same files, so adding another line may not be needed here.

[blakejohnson]

Well, that doesn't look good (from AppVeyor x86_64):

Assertion failed!
Program: C:\projects\julia\usr\bin\julia.exe
File: C:/projects/julia/src/gc.c, Line 607
Expression: maybe && "find_region failed"

[nalimilan]

I think that error happened before for other PRs.

[vtjnash]

lgtm

[blakejohnson]

I'll try restarting AppVeyor, then.

[tkelman]

Should we rebuild the Windows (and OSX if Travis were running all the tests there) LLVM binaries then add the reduced test case for this?

[blakejohnson]

@tkelman we should definitely rebuild the LLVM binaries. I'm still not sure what it is exactly about the test case from #14704 that triggers the problem, other than having somewhat complicated control flow. But, we could certainly add it.

[bicycle1885]

+1 for rebuilding the LLVM binaries if it will solve Travis-CI failures I encountered due to the bug.

[tkelman]

I'm referring to the binaries of just llvm itself, that only get used for OSX Travis (homebrew-julia bottles) and AppVeyor testing of Julia base. The nightly binaries of Julia get rebuilt continuously from a source build, so those should pick up this change automatically as long as buildbot status is all okay. See status.julialang.org for the age/commit of the latest nightlies.

[bicycle1885]

I see. Thank you!

[tkelman]

This patch is now included on both appveyor and the homebrew bottle, so we can add any tests that depend on it. Maybe we should key off of whether a system LLVM is used somehow, and only run the test when we know we're using our own LLVM build.

[nalimilan]

Maybe we should key off of whether a system LLVM is used somehow, and only run the test when we know we're using our own LLVM build.

On the other hand, not checking for known-buggy behavior isn't great. I know I'll (try to) ship that patch in Fedora rather than failing, and I think we'd rather have other maintainers do the same. If they really can't, they can still disable the tests. Cc: @JuliaLang/distro-packagers

@blakejohnson Do you plan to make a PR to add tests?

[tkelman]

Annoyingly no one upstream has reviewed http://reviews.llvm.org/D16505 yet.

[tkelman]

This did get reviewed upstream but needs to add a test case there. Let's try not to carry this patch forever if we can get it merged upstream.

[Keno]

Patch committed upstream as http://reviews.llvm.org/rL275239.