[PATCH] [OPENMP] A helper for marking intstructions with llvm.mem.parallel_loop_access

[Alexander Musman]

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 <hfinkel at anl.gov>:

>
>
> ----- Original Message -----
> > From: "Tyler Nowicki" <tnowicki at apple.com>
> > To: "Alexander Musman" <alexander.musman at gmail.com>
> > Cc: "Douglas Gregor" <dgregor at apple.com>, rjmccall at gmail.com, "Richard
> Smith" <richard at metafoo.co.uk>, "Alexey
> > Bataev" <a.bataev at hotmail.com>, "Hal Finkel" <hfinkel at anl.gov>,
> gribozavr at gmail.com, cbergstrom at pathscale.com,
> > "renato golin" <renato.golin at linaro.org>, "llvm cfe" <
> cfe-commits at cs.uiuc.edu>
> > 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
> > <alexander.musman at gmail.com> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140508/c627e2ee/attachment.html>