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

Refactor Expr(:simdloop) to Expr(:loopinfo, ...) #31095

Merged
merged 1 commit into from
Mar 13, 2019
Merged

Conversation

vchuravy
Copy link
Member

  • Expr(:loopinfo, Symbol("julia.simdloop")) replaces Expr(:simdloop, false)
  • Expr(:loopinfo, Symbol("julia.simdloop"), Symbol("julia.ivdep")) replaces Expr(:simdloop, true)

While currently not directly exposed, users can now pass other loopinfo metadata to LLVM.
Codegen attaches the loop metadata to the call to the marker and the LowerSIMD pass consumes that metadata and
attaches it to the loop.

@eval function unroll(X)
           acc = 0.0
           @inbounds for i in 1:length(X)
               acc += X[i]
               $(Expr(:loopinfo, (Symbol("llvm.loop.unroll.count"), 4),
                                 (Symbol("llvm.loop.vectorize.enable"), false)))
           end
           return acc
       end

I needed this for #31086, this also removes unneeded and untested machinery from llvm-simdloop.cpp.

@vchuravy vchuravy added compiler:codegen Generation of LLVM IR and native code needs tests Unit tests are required for this change needs docs Documentation for this change is required labels Feb 16, 2019
@vchuravy vchuravy requested a review from Keno February 16, 2019 21:55
@KristofferC KristofferC added the needs nanosoldier run This PR should have benchmarks run on it label Feb 16, 2019
@vchuravy vchuravy removed the needs tests Unit tests are required for this change label Mar 12, 2019
@vchuravy vchuravy marked this pull request as ready for review March 12, 2019 19:43
@vchuravy
Copy link
Member Author

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

- `Expr(:loopinfo, Symbol("julia.simdloop"))` replaces
  `Expr(:simdloop, false)`
- `Expr(:loopinfo, Symbol("julia.simdloop"), Symbol("julia.ivdep"))` replaces
  `Expr(:simdloop, true)`

While currently not directly exposed users can now pass other
[loopinfo](https://llvm.org/docs/LangRef.html#llvm-loop) metadata to
LLVM.
@vchuravy vchuravy removed needs docs Documentation for this change is required needs nanosoldier run This PR should have benchmarks run on it labels Mar 12, 2019
@nanosoldier
Copy link
Collaborator

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

@vchuravy
Copy link
Member Author

vchuravy commented Mar 13, 2019

Nanosoldier seems fine (if this would lead to failed vectorisation I would expect slow downs on the oder of 4x)

@Keno good to merge or so you have comments about the current implementation?

@Keno
Copy link
Member

Keno commented Mar 13, 2019

Fine with me.

@vtjnash
Copy link
Member

vtjnash commented Mar 14, 2019

/Users/jameson/julia/src/codegen.cpp:4109:24: warning: variable 'MD' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
            } else if (jl_is_tuple(arg)) {
                       ^~~~~~~~~~~~~~~~
./julia.h:959:30: note: expanded from macro 'jl_is_tuple'
#define jl_is_tuple(v)       (((jl_datatype_t*)jl_typeof(v))->name == jl_tuple_typename)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/jameson/julia/src/codegen.cpp:4127:17: note: uninitialized use occurs here
            if (MD)
                ^~
/Users/jameson/julia/src/codegen.cpp:4109:20: note: remove the 'if' if its condition is always true
            } else if (jl_is_tuple(arg)) {
                   ^~~~~~~~~~~~~~~~~~~~~~
/Users/jameson/julia/src/codegen.cpp:4104:25: note: initialize the variable 'MD' to silence this warning
            Metadata *MD;
                        ^
                         = nullptr
1 warning generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants