[PATCH] [Polly] [IslCodeGenerator] Add OpenMP support

Tobias Grosser tobias at grosser.es
Thu Nov 6 11:20:37 PST 2014


Sorry for the delay. I just pushed out a new patch which aims to address 
your comments.

On 29.09.2014 19:13, Johannes Doerfert wrote:
> I still haven't looked at the test cases but I commented the rest of the patch. We should add more test cases and review all of them later.
>
> ================
> Comment at: lib/CodeGen/IslCodeGeneration.cpp:457
> @@ +456,3 @@
> +  Body = isl_ast_node_for_get_body(For);
> +
> +  Init = isl_ast_node_for_get_init(For);
> ----------------
> remove the newline
>
> ================
> Comment at: lib/CodeGen/IslCodeGeneration.cpp:460
> @@ +459,3 @@
> +  Inc = isl_ast_node_for_get_inc(For);
> +  Iterator = isl_ast_node_for_get_iterator(For);
> +  IteratorID = isl_ast_expr_get_id(Iterator);
> ----------------
> I would call it It and ItId but that's not important.
>
> ================
> Comment at: lib/CodeGen/IslCodeGeneration.cpp:505
> @@ +504,3 @@
> +
> +  clearDomtree((*LoopBody).getParent()->getParent(),
> +               P->getAnalysis<DominatorTreeWrapperPass>().getDomTree());
> ----------------
> I still do not know why we need this, shouldn't we just create the domtree on the fly (when we create the subfunc) correctly?

I added a comment documenting this.

> ================
> Comment at: lib/CodeGen/IslCodeGeneration.cpp:525
> @@ +524,3 @@
> +
> +  if (IslAstInfo::isOutermostParallel(For) &&
> +      !IslAstInfo::isReductionParallel(For)) {
> ----------------
> You already mentioned --enable-polly-openmp but I would like to rename that option when we are at it, e.g.,
>    --polly-enable-parallel

I used -polly-parallel for now.

> ================
> Comment at: lib/CodeGen/IslCodeGeneration.cpp:604
> @@ -460,2 +603,3 @@
>     }
> +  VMap.insert(ValueMap.begin(), ValueMap.end());
>
> ----------------
> This needs some docu, I don't know why we need this here? Whats in which map?

Added a comment.

> ================
> Comment at: lib/CodeGen/IslCodeGeneration.cpp:770
> @@ -625,2 +769,3 @@
>     virtual void getAnalysisUsage(AnalysisUsage &AU) const {
> +    AU.addRequired<DataLayoutPass>();
>       AU.addRequired<DominatorTreeWrapperPass>();
> ----------------
> Where is that one used? Was it missing before?

This is needed for the OpenMP loop generator. We now pass the data 
layout explicitly to make the use directly visible.

Tobias




More information about the llvm-commits mailing list