search

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

@par test broken #83

Closed ehsantn closed 8 years ago

ehsantn commented 8 years ago

@par test is broken. Travis was ignoring it since debug environment variable was not set (fixed now). It is probably coming from the recent changes in ParallelIR.

ninegua commented 8 years ago

I can confirm it still worked as of commit ea3c30882ee440bcce2be6b4a77d2f578c3eecf7

ehsantn commented 8 years ago

The problem seems to be that :n is not in the escaping variables of the inner lambda. @ninegua could you take a look?

ninegua commented 8 years ago

The issue is in commit 6c59cce4788c9c8e1ce7b461d7d69a38d63560f7. Believe you can't do this correlation with arrays in the outer lambda because it will pull in variables only defined in the outer lambda as the size for array used in the inner lambda. The inner lambda never referenced n but due to the correlation, n is pulled in.

This kind of correlation is in general unsafe, largely due to the lack of SSA. But the important thing to remember is that the definition of inner lambda is no indication of when it is executed/called. The correlation that holds at definition time may not hold at running time. The lambda could be invoked out-of-scope or anywhere. So I think it is safer to wait until the inner lambdas is merged into the outer before we establish the correlations. What do you think? @DrTodd13 @ehsantn

ehsantn commented 8 years ago

Can't we solve the lack of SSA problem for equivalences for now by being conservative and not establishing correlation when an array has different possible allocation points? Implementation of this seems easy to me in the AST walk code.

A correlation is established only if the sizes are constant integers or assigned once. Therefore, it doesn't matter when the inner lambda is called. Is there a case where the size might be assigned after inner lambda is called?

The solution for this example seems to me is to merge correlation size variables into inner lambda since they are constants.

ninegua commented 8 years ago

Why was 6c59cce necessary in the first place? All inner lambdas that should be inlined will be inlined once domain operators are lowered into parfors. We may still have nested parfors, but variables in them are considered to be in the same scope.

ehsantn commented 8 years ago

The problem is that all optimizations happen before lowering to parfors.

ehsantn commented 8 years ago

Oh by inner lambda I mean the body of mmap and mmap! nodes. Can they be called somewhere else?

ninegua commented 8 years ago

The subject is rather delicate. There is at least one known issue #78 with it. Even if we can assume all the safety cautions (constant or assigned once, array object vs. variable, etc.) in place, the fix is not just pulling in the size symbol as an escaping variable because it could clash with a locally defined variable. Probably need to generate a fresh local var to put in the symbol_array_correlation array.

Also, you are reusing domain_lambda (actually its linfo) in the setEscCorrelations! function, which is wrong. Because the linfo no longer corresponds to the what is in the ast, which was created in lambdaFromDomainLambda. So it is best to create a new linfo from ast again before calling setEscCorrelations!.

ninegua commented 8 years ago

In order to get travis back, I pushed a fix f0cd475558257f6ab6914f245a6a2d8d750b7b2b . It throws an error when there is a conflict with local variable names. We'll have to revisit this topic when that happens.