[PATCH] #pragma vectorize

Richard Smith richard at metafoo.co.uk
Mon Jun 2 17:32:39 PDT 2014


On Mon, Jun 2, 2014 at 4:18 PM, Tyler Nowicki <tnowicki at apple.com> wrote:

> Hi Richard,
>
> Thanks again for the review. I’m glad to hear its getting closer. I’ve
> applied your suggestions. Here is the updated patch.
>
> > Once you move to supporting arbitrary constant expressions as the
> argument, I think you may want to split this into two separate Attrs (one
> for vectorize and interleave, that have a BoolArgument, and one for
> vectorize_width and interleave_count, that have an ExprArgument).
> >
> > I also think we should have different spellings for these different enum
> values, once we have proper pragma spelling support:
> >
> >   let Spellings = [Pragma<"clang", "loop", "vectorize_width">,
> >                    Pragma<"clang", "loop", "interleave_count">];
> >   let Args = [ExprArgument<"Value">];
> >
> > ... but that can all be deferred for a later patch.
>
> I like the idea of separating the LoopHintAttrs that may make it easier to
> support c++11 spellings as well.


+    const char *MetadataNames[] = {
+        "llvm.vectorizer.width", "llvm.vectorizer.width",
+        "llvm.vectorizer.unroll", "llvm.vectorizer.unroll"};
+
+    switch (Option) {
+    case LoopHintAttr::Vectorize:
+    case LoopHintAttr::Interleave:

Please remove the array and set the strings inside the switch, so you're
not depending on the values of the enumerators here.

Other than that, LGTM; thank you for your patience!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140602/04d87ddf/attachment.html>


More information about the cfe-commits mailing list