[PATCH] #pragma vectorize

[Richard Smith]

On Sun, Jun 1, 2014 at 7:39 PM, Tyler Nowicki <tnowicki at apple.com> wrote:

> Hi Richard,
>
> Thanks for the detailed review!
>
> I went with the last syntax you suggested. Seems simple to me and easily
> extendable: #pragma clang loop vectorize_width(2) interleave_count(4).
>
> Also it helped clean up a lot of the ambiguity you talked about in the
> LoopHintAttr. A single integer Value has replaced the Kind
> (Enable/Disable/Value) and a integer value there previously. Hopefully it
> won’t be too difficult to support constant expressions, and non-type
> template parameters. I’ll work on those patches next.
>
> I also improved the incompatibility testing as well. I realized there were
> several cases my previous test wasn’t identifying.
>
>
> Objective C is a proper superset of C, so yes. I don't know Objective C
> very well either, and it might be the case that it would never be possible
> to vectorize its for loop, because it's too dynamic.
>
>
> Ok, its on my list.
>
>
> I think you want (and what the approach I described above permits) is to
> callParseConstantExpression.
>
>
> I think I understand now what you are suggesting. Your thinking that the
> expression may include more than one token. I gave it a try, but I wasn’t
> able to get it to work. The expression returned by ParseConstantExpression
> was always invalid. Perhaps I didn’t implement it correctly. I’m capturing
> all of the tokens between the brackets and then when handling the annotated
> token I reinserted them into the stream and called ParseConstantExpression.
> I tried it with and without the brackets and with and without a trailing
> eof token. The LHS and Res expressions parsed in ParseConstantExpression
> were also always invalid <<NULL>>. Lets leave this and other constant
> expression work for a later patch.
>

OK.


Here is the updated patch.
>

I think this is very close to being ready.

+  let Args = [EnumArgument<"Option", "OptionType",
+                          ["vectorize", "vectorize_width", "interleave",
"interleave_count"],
+                          ["Vectorize", "VectorizeWidth", "Interleave",
"InterleaveCount"]>,
+              DefaultIntArgument<"Value", 1>];

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.


+    const char *MetadataNames[] = {
+        "llvm.vectorizer.width", "llvm.vectorizer.width",
+        "llvm.vectorizer.unroll", "llvm.vectorizer.unroll"};
+    if (ValueInt == 1 && (Option == LoopHintAttr::Vectorize ||
+                          Option == LoopHintAttr::Interleave)) {
+      Name = llvm::MDString::get(Context, "llvm.vectorizer.enable");
+      Value = Builder.getTrue();
+    } else {
+      if (ValueInt == 0)
+        ValueInt = 1; // Vectorization/interleaving is disabled.
+      Name = llvm::MDString::get(Context, MetadataNames[Option]);
+      Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
+    }

Please change this to a 'switch' to make this code more robust against
enumerators being added later.


+///  loop-hint:
+///    'vectorize' '(' loop-hint-value ')'
+///    'interleave' '(' loop-hint-value ')'
+///
+///  loop-hint-value:
+///    'enable'
+///    'disable'
+///    constant-expression

This needs to be updated.


+// CHECK: br i1 %cmp, label %while.body, label %while.end, !llvm.loop !1
+// CHECK: br i1 %cmp, label %do.body, label %do.end, !llvm.loop !5
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !7
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !8
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !11
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !13
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !14
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !16

I believe these tests will fail on a NDEBUG build, because we don't give
names to values and basic blocks there. What we'd usually do is something
like this:

+void while_test(int *List, int Length) {
+  // CHECK: define {{.*}} @_Z10while_test
+  int i = 0;
+
+#pragma clang loop vectorize(enable)
+#pragma clang loop interleave_count(4)
+#pragma clang loop vectorize_width(4)
+  while (i < Length) {
+    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop
![[LOOP_1:.*]]
+    List[i] = i * 2;
+    i++;
+  }
+}
[... at end of file ...]
+// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata
![[WIDTH_4:.*]], metadata ![[UNROLL_4:.*]], metadata ![[ENABLE_1:.*]]}
+// CHECK: ![[WIDTH_4]] = metadata !{metadata !"llvm.vectorizer.width", i32
4}
[...]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140602/91fa32b0/attachment.html>