Hi Tyler, Hal, > Looks like this implementation is an alternative to the patch I am proposing.
I agree with Hal, I think both approaches (#pragma loop vectorize and #pragma omp simd) can co-exist. #pragma omp simd follows Openmp standard and #pragma loop vectorize has more freedom to add any hints specific for tuning vectorizer. 1) Actually it is part of AST - for example, see test/OpenMP/simd_ast_print.cpp for serialization and ast printing. There is a separate AST node (see OMPSimdDirective declaration in include/clang/AST/StmtOpenMP.h). 2) Templates - this part is also implemented, there is a test for non-type parameter in 'safelen' clause in the same file. If you are interested how template instatiation is done, I suggest to look at TransformOMPSafelenClause (in lib/Sema/TreeTransform.h). (I'm not sure that the same approach will work for pragma loop vectorize though, because it uses attributes while OMPSimdDirective has a separate AST node). 3) No, it is not a separate pass - routine InsertHelper is called for each instruction from IR Builder, so that, when it is in the "parallel" loop body, it will mark load/store instructions with llvm.mem.parallel_loop_access. Thanks, Alexander 2014-05-07 22:47 GMT+04:00 Hal Finkel <[email protected]>: > > > ----- Original Message ----- > > From: "Tyler Nowicki" <[email protected]> > > To: "Alexander Musman" <[email protected]> > > Cc: "Douglas Gregor" <[email protected]>, [email protected], "Richard > Smith" <[email protected]>, "Alexey > > Bataev" <[email protected]>, "Hal Finkel" <[email protected]>, > [email protected], [email protected], > > "renato golin" <[email protected]>, "llvm cfe" < > [email protected]> > > Sent: Wednesday, May 7, 2014 12:18:17 PM > > Subject: Re: [PATCH] [OPENMP] A helper for marking intstructions with > llvm.mem.parallel_loop_access > > > > Hi Alexander, > > > > Looks like this implementation is an alternative to the patch I am > > proposing. > > Just to provide a small amount of additional context: > > OpenMP 4 specifies standard pragmas for loop vectorization. These differ, > generically, from vendor-specific vectorization pragmas because they 1) > assert safety and must work (or produce an error; they are not just hints) > and 2) the standard specifies a restricted syntax for the loops which the > frontend must check. This is not an alternative to what you've proposed: we > need both! That having been said, we should obviously unify the > implementation to the extent possible. > > -Hal > > > I’m pretty sure I also saw something like this on the > > list before. There are a few things I am concerned about with this > > approach. I am new to clang development though so please let me know > > what I misunderstood anything. > > > > 1. The pragma is not part of the AST. This makes the pragmas hidden > > to any passes over the AST. That may cause problems for features > > like serialization/deserialization and ast-print. If the pragma is > > an AST node those are easily supported. > > > > 2. Templates. I know there is a lot of interest in allowing the loop > > vectorization pragmas to take non-type template parameters. It has > > been a much requested feature on my patch. One of the challenges I > > am finding is that the non-type template parameters should be > > resolved on template instantiation. This probably has to occur when > > AST nodes are visited during semantic analysis. A few people have > > commented that they don’t want it in CodeGen because that would mean > > the verification of the input values would occur in codegen as well > > (see link [1] below). I’m still trying to figure this one out. But I > > am wondering how would you do it? > > > > 2. Efficiency. It looks like you have to make an extra pass over > > every loop.cond and loop.body even if metadata was not attached to > > those loops? Or did I miss something? > > > > As I said, I’m new to clang so if I misunderstood anything please let > > me know. > > > > Tyler > > > > [1] - > > > http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140505/104925.html > > > > On May 7, 2014, at 6:13 AM, Alexander Musman > > <[email protected]> wrote: > > > > > Hi doug.gregor, rjmccall, rsmith, ABataev, hfinkel, gribozavr, > > > cbergstrom, rengolin, TylerNowicki, > > > > > > This patch adds a helper class (CGLoopInfo) for marking memory > > > instructions with llvm.mem.parallel_loop_access metadata. > > > It also adds a simple initial version of codegen for pragma omp > > > simd (it will change in the future to support all the clauses). > > > > > > http://reviews.llvm.org/D3644 > > > > > > Files: > > > lib/CodeGen/CGBuilder.h > > > lib/CodeGen/CGLoopInfo.cpp > > > lib/CodeGen/CGLoopInfo.h > > > lib/CodeGen/CGStmt.cpp > > > lib/CodeGen/CGStmtOpenMP.cpp > > > lib/CodeGen/CMakeLists.txt > > > lib/CodeGen/CodeGenFunction.cpp > > > lib/CodeGen/CodeGenFunction.h > > > test/OpenMP/simd_metadata.c > > > <D3644.9161.patch> > > > > > > -- > Hal Finkel > Assistant Computational Scientist > Leadership Computing Facility > Argonne National Laboratory >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
