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

LLVM generates bad code on newer architercures #19976

Closed
pabloferz opened this issue Jan 11, 2017 · 2 comments · Fixed by #22205
Closed

LLVM generates bad code on newer architercures #19976

pabloferz opened this issue Jan 11, 2017 · 2 comments · Fixed by #22205
Labels
performance Must go faster upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@pabloferz
Copy link
Contributor

pabloferz commented Jan 11, 2017

Didn't find an issue for this. Cross-reference: #19259 (comment)

I believe some of the performance "noisy" regressions seen lately are caused by this.

On my machine (Haswell) the pisum() benchmark on master gives

julia> @benchmark pisum()
BenchmarkTools.Trial: 
  memory estimate:  0.00 bytes
  allocs estimate:  0
  --------------
  minimum time:     33.362 ms (0.00% GC)
  median time:      33.445 ms (0.00% GC)
  mean time:        33.532 ms (0.00% GC)
  maximum time:     34.761 ms (0.00% GC)
  --------------
  samples:          150
  evals/sample:     1
  time tolerance:   5.00%
  memory tolerance: 1.00%

while on 0.5 I get

julia> @benchmark pisum()
BenchmarkTools.Trial: 
  memory estimate:  0.00 bytes
  allocs estimate:  0
  --------------
  minimum time:     7.548 ms (0.00% GC)
  median time:      7.571 ms (0.00% GC)
  mean time:        7.611 ms (0.00% GC)
  maximum time:     8.404 ms (0.00% GC)
  --------------
  samples:          657
  evals/sample:     1
  time tolerance:   5.00%
  memory tolerance: 1.00%

Even when both show practically the same LLVM code.

cc @yuyichao

@pabloferz pabloferz added performance Must go faster regression Regression in behavior compared to a previous version labels Jan 11, 2017
@vchuravy
Copy link
Member

A direct way of seeing the effect on the same version of Julia is:

julia -C core2 --precompiled=no pisum.jl
@benchmark(pisum()) = BenchmarkTools.Trial: 
  memory estimate:  0.00 bytes
  allocs estimate:  0
  --------------
  minimum time:     19.512 ms (0.00% GC)
  median time:      19.795 ms (0.00% GC)
  mean time:        19.797 ms (0.00% GC)
  maximum time:     21.656 ms (0.00% GC)
  --------------
  samples:          253
  evals/sample:     1
  time tolerance:   5.00%
  memory tolerance: 1.00%

vs:

julia -C haswell --precompiled=no pisum.jl
@benchmark(pisum()) = BenchmarkTools.Trial: 
  memory estimate:  0.00 bytes
  allocs estimate:  0
  --------------
  minimum time:     37.291 ms (0.00% GC)
  median time:      37.888 ms (0.00% GC)
  mean time:        37.931 ms (0.00% GC)
  maximum time:     39.078 ms (0.00% GC)
  --------------
  samples:          132
  evals/sample:     1
  time tolerance:   5.00%
  memory tolerance: 1.00%

@vchuravy vchuravy added the upstream The issue is with an upstream dependency, e.g. LLVM label Jan 11, 2017
@yuyichao yuyichao removed the regression Regression in behavior compared to a previous version label Jan 11, 2017
@yuyichao
Copy link
Contributor

It's not a regression, I've checked that both LLVM 3.3 and 3.7.1 gives the same bad code when compiling with avx enabled.

This might have been fixed on LLVM trunk right after 3.9 is branched and I'm trying to backport the patch.

yuyichao added a commit that referenced this issue Jun 3, 2017
This fixes a bug in the patch that fixes #19976 causing encoding error on 32bit x86
and segfault when AVX/AVX2 is enabled.

Ref LLVM bug report https://bugs.llvm.org//show_bug.cgi?id=29010
LLVM commit llvm-mirror/llvm@83260f2
Also ref where I saw this issue in #21849 (comment)
tkelman pushed a commit that referenced this issue Jun 6, 2017
This fixes a bug in the patch that fixes #19976 causing encoding error on 32bit x86
and segfault when AVX/AVX2 is enabled.

Ref LLVM bug report https://bugs.llvm.org//show_bug.cgi?id=29010
LLVM commit llvm-mirror/llvm@83260f2
Also ref where I saw this issue in #21849 (comment)

(cherry picked from commit 5a42474)
ref #22205
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants