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

AstWalkCallback for DomainLambda #133

Closed ehsantn closed 7 years ago

ehsantn commented 7 years ago

AstWalkCallback in DomainIR doesn't recurse inside the body. Therefore, AST walks like copy propagation don't go inside.

ninegua commented 7 years ago

So is this about renaming an escaping variable? Can you give me an example of where you need to do this?

ehsantn commented 7 years ago

No, it's not renaming. For example, copy propagation handles DomainLambda in here and returns ASTWALK_RECURSE. But DomainIR doesn't go inside body here and just returns. A good example is the kernel density code where some obvious constants are not replaced.

ninegua commented 7 years ago

I don't think AstWalker ever goes into nested lambda expressions. We made a special case for replaceExprWithDict! to explicitly deal with escaping variables. If this is the function you use in copy propagation, I can make it accommodate DomainLambda too.

ninegua commented 7 years ago

Wait, I believe replaceExprWithDict! is already setup to handle DomainLambda just fine. The relevant code is here. So copy propagation has to use replaceExprWithDict! if it is not doing so.

ehsantn commented 7 years ago

replaceExprWithDict is a special case. Is traversing DomainLambda's body fundamentally wrong?

ninegua commented 7 years ago

I wouldn't say all nested traversals are wrong, but variable name clashing is a delicate matter and must be handled with care, which is often times specific to use scenario. So it is best not to generalize the logic into generic traversal code, as most traversals are not interested in nested case, and would be wrong otherwise if not handled carefully.

ehsantn commented 7 years ago

Ok, makes sense. I will use special code then.