IntelLabs / ParallelAccelerator.jl

The ParallelAccelerator package, part of the High Performance Scripting project at Intel Labs
BSD 2-Clause "Simplified" License
294 stars 32 forks source link

Size inference in a situation with multiple assignments #78

Closed ninegua closed 8 years ago

ninegua commented 8 years ago

The following program crashes:

using ParallelAccelerator

@acc function f(n)
 A=ones(n)
 B=zeros(0)
 for i = 1:3
    if i==1 
       B = zeros(n)
    else
       B = B.+ A
    end
 end
 return sum(B)
end

println("f(10)=", @noacc f(10))
println("f(10)=", f(10))

The generated code shows that the initialization of A and B are fused together and A's size is used, so B is written to out of bound. It's another place where the lack of SSA is causing trouble.

BTW, this is not an artificially made-up program just to confuse our compiler. Somehow opt-flow was written this way, and I had a long debug session only to track it down to zero-sized initialization: d8297da46c0e736025b7e5372cdb06ca0fed3e81

DrTodd13 commented 8 years ago

How urgent is this? Can it wait for a couple weeks?

ninegua commented 8 years ago

I won't say it is urgent as we can always re-write a program to deal with it. But it is an important issue to keep in mind, i.e., substituting a variable by its definition (or the equivalent of it, like here we assumed size is associated with a variable) is very fragile without SSA. Everyone working on our compiler should watch out for potential issues. @ehsantn @lkuper @ChunlingHu

I certainly hope the coming changes to Julia AST can make our job a little easier in this aspect.

DrTodd13 commented 8 years ago

There is something else probably higher priority so I will come back to this in a bit. My general plan is to add a scan of the AST looking for arrays whose size could potentially be changed in multiple statements. For each such array found, I'm thinking of changing that array's equivalence class to some negative value. Such negative valued equivalence classes have the property that you can never merge with them. Thus, the array will be unique and then won't fuse with anything. This over-strictness is probably the best we can do without SSA or some such equivalent.

DrTodd13 commented 8 years ago

Should be fixed in d588718. @ninegua please confirm and close this ticket.

DrTodd13 commented 8 years ago

This issue seems to be different than the one @ninegua showed me with optical flow. The issue in optical flow was trying to replace array length expressions with their size initializers from symbol_correlation table but if some array is multiply defined then at the moment the first size encountered goes in symbol_correlation table. Opt-flow was doing something weird with 0 size initializations at first and then redefining later. My latest check-in seems to fix this problem.

Please try again @ninegua .

ninegua commented 8 years ago

Fix verified. Thanks! @DrTodd13