<div dir="ltr"><div>Hi Tyler, Hal,</div><div><br></div><div>> Looks like this implementation is an alternative to the patch I am proposing.</div><div><br></div><div>I agree with Hal, I think both approaches (#pragma loop vectorize and #pragma omp simd) can co-exist.</div>
<div>#pragma omp simd follows Openmp standard and #pragma loop vectorize has more freedom to add any hints specific for tuning vectorizer.</div><div><br></div><div>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).</div>
<div><br></div><div>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).</div>
<div><br></div><div>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.</div>
<div><br></div><div>Thanks,</div><div>Alexander</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-05-07 22:47 GMT+04:00 Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
<br>
----- Original Message -----<br>
> From: "Tyler Nowicki" <<a href="mailto:tnowicki@apple.com">tnowicki@apple.com</a>><br>
> To: "Alexander Musman" <<a href="mailto:alexander.musman@gmail.com">alexander.musman@gmail.com</a>><br>
> Cc: "Douglas Gregor" <<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>>, <a href="mailto:rjmccall@gmail.com">rjmccall@gmail.com</a>, "Richard Smith" <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>>, "Alexey<br>

> Bataev" <<a href="mailto:a.bataev@hotmail.com">a.bataev@hotmail.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>, <a href="mailto:gribozavr@gmail.com">gribozavr@gmail.com</a>, <a href="mailto:cbergstrom@pathscale.com">cbergstrom@pathscale.com</a>,<br>

> "renato golin" <<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>>, "llvm cfe" <<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>><br>
> Sent: Wednesday, May 7, 2014 12:18:17 PM<br>
> Subject: Re: [PATCH] [OPENMP] A helper for marking intstructions with llvm.mem.parallel_loop_access<br>
><br>
> Hi Alexander,<br>
><br>
> Looks like this implementation is an alternative to the patch I am<br>
> proposing.<br>
<br>
</div>Just to provide a small amount of additional context:<br>
<br>
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.<br>

<br>
 -Hal<br>
<div class="HOEnZb"><div class="h5"><br>
> I’m pretty sure I also saw something like this on the<br>
> list before. There are a few things I am concerned about with this<br>
> approach. I am new to clang development though so please let me know<br>
> what I misunderstood anything.<br>
><br>
> 1. The pragma is not part of the AST. This makes the pragmas hidden<br>
> to any passes over the AST. That may cause problems for features<br>
> like serialization/deserialization and ast-print. If the pragma is<br>
> an AST node those are easily supported.<br>
><br>
> 2. Templates. I know there is a lot of interest in allowing the loop<br>
> vectorization pragmas to take non-type template parameters. It has<br>
> been a much requested feature on my patch. One of the challenges I<br>
> am finding is that the non-type template parameters should be<br>
> resolved on template instantiation. This probably has to occur when<br>
> AST nodes are visited during semantic analysis. A few people have<br>
> commented that they don’t want it in CodeGen because that would mean<br>
> the verification of the input values would occur in codegen as well<br>
> (see link [1] below). I’m still trying to figure this one out. But I<br>
> am wondering how would you do it?<br>
><br>
> 2. Efficiency. It looks like you have to make an extra pass over<br>
> every loop.cond and loop.body even if metadata was not attached to<br>
> those loops? Or did I miss something?<br>
><br>
> As I said, I’m new to clang so if I misunderstood anything please let<br>
> me know.<br>
><br>
> Tyler<br>
><br>
> [1] -<br>
> <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140505/104925.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140505/104925.html</a><br>
><br>
> On May 7, 2014, at 6:13 AM, Alexander Musman<br>
> <<a href="mailto:alexander.musman@gmail.com">alexander.musman@gmail.com</a>> wrote:<br>
><br>
> > Hi doug.gregor, rjmccall, rsmith, ABataev, hfinkel, gribozavr,<br>
> > cbergstrom, rengolin, TylerNowicki,<br>
> ><br>
> > This patch adds a helper class (CGLoopInfo) for marking memory<br>
> > instructions with llvm.mem.parallel_loop_access metadata.<br>
> > It also adds a simple initial version of codegen for pragma omp<br>
> > simd (it will change in the future to support all the clauses).<br>
> ><br>
> > <a href="http://reviews.llvm.org/D3644" target="_blank">http://reviews.llvm.org/D3644</a><br>
> ><br>
> > Files:<br>
> >  lib/CodeGen/CGBuilder.h<br>
> >  lib/CodeGen/CGLoopInfo.cpp<br>
> >  lib/CodeGen/CGLoopInfo.h<br>
> >  lib/CodeGen/CGStmt.cpp<br>
> >  lib/CodeGen/CGStmtOpenMP.cpp<br>
> >  lib/CodeGen/CMakeLists.txt<br>
> >  lib/CodeGen/CodeGenFunction.cpp<br>
> >  lib/CodeGen/CodeGenFunction.h<br>
> >  test/OpenMP/simd_metadata.c<br>
> > <D3644.9161.patch><br>
><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div>